-
Notifications
You must be signed in to change notification settings - Fork 2
Delayed postprocessing timeout logic #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3314c53
Delayed postprocessing timeout logic
denizergonul ad8c6ec
Nested class PostprocessManager with unit test
denizergonul 7ad14bb
Rename and cap fix
denizergonul 6bcf425
Variable rename
denizergonul 2209892
Lint updates
denizergonul deddaa8
Uninitialized variables fix
denizergonul 7b4e158
Added SupportsDelayedPostprocessing concept
denizergonul 6e71a70
Postprocess coroutine timeout test
denizergonul 665b0be
Skipping timeout coroutine if no max wait set.
alessandrothea 3bd3e89
Adding a toy test app to examine the timeout coroutine logic in isola…
alessandrothea ed37033
Adding new test app to makefile
alessandrothea 6148b21
Logging pp timeout at run start for testing
alessandrothea e7df040
Re-arranged postprocessing timeout handling
alessandrothea fb18b44
Removing unused variable
alessandrothea File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
include/datahandlinglibs/models/detail/SkipListLatencyBufferModel.hxx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /** | ||
| * @file UnitTestUtilities.hpp Unit test helper classes | ||
| * | ||
| * This is part of the DUNE DAQ Application Framework, copyright 2020. | ||
| * Licensing/copyright details are in the COPYING file that you should have | ||
| * received with this code. | ||
| */ | ||
|
|
||
| #ifndef DATAHANDLINGLIBS_INCLUDE_DATAHANDLINGLIBS_TESTUTILS_UNITTESTUTILITIES_HPP | ||
| #define DATAHANDLINGLIBS_INCLUDE_DATAHANDLINGLIBS_TESTUTILS_UNITTESTUTILITIES_HPP | ||
|
|
||
| #include "datahandlinglibs/models/DataHandlingModel.hpp" | ||
| #include "datahandlinglibs/models/DefaultRequestHandlerModel.hpp" | ||
| #include "datahandlinglibs/models/TaskRawDataProcessorModel.hpp" | ||
|
|
||
| namespace dunedaq { | ||
| namespace datahandlinglibs { | ||
| namespace unittest { | ||
|
|
||
| template<typename ReadoutType, | ||
| typename RequestHandlerType, | ||
| typename LatencyBufferType, | ||
| typename RawDataProcessorType, | ||
| typename InputDataType = ReadoutType> | ||
| class MockDataHandlingModel | ||
| : public DataHandlingModel<ReadoutType, RequestHandlerType, LatencyBufferType, RawDataProcessorType, InputDataType> | ||
| { | ||
| public: | ||
| using Base = | ||
| DataHandlingModel<ReadoutType, RequestHandlerType, LatencyBufferType, RawDataProcessorType, InputDataType>; | ||
| using Base::Base; | ||
| using Base::PostprocessScheduleAlgorithm; | ||
| }; | ||
|
|
||
| } // namespace unittest | ||
| } // namespace datahandlinglibs | ||
| } // namespace dunedaq | ||
|
|
||
| #endif // DATAHANDLINGLIBS_INCLUDE_DATAHANDLINGLIBS_TESTUTILS_UNITTESTUTILITIES_HPP |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| /** | ||
| * @file datahandlinglibs_DataHandlingModel_test.cxx Unit Tests for DataHandlingModel | ||
| * | ||
| * This is part of the DUNE DAQ Application Framework, copyright 2020. | ||
| * Licensing/copyright details are in the COPYING file that you should have | ||
| * received with this code. | ||
| */ | ||
|
|
||
| #define BOOST_TEST_MODULE datahandlinglibs_DataHandlingModel_test // NOLINT | ||
|
|
||
| #include "boost/test/unit_test.hpp" | ||
|
|
||
| #include "datahandlinglibs/ReadoutTypes.hpp" | ||
| #include "datahandlinglibs/models/SkipListLatencyBufferModel.hpp" | ||
| #include "datahandlinglibs/testutils/UnitTestUtilities.hpp" | ||
|
|
||
| #include <memory> | ||
| #include <utility> | ||
|
|
||
| BOOST_AUTO_TEST_SUITE(datahandlinglibs_DataHandlingModel_test) | ||
|
|
||
| using namespace dunedaq::datahandlinglibs; | ||
|
|
||
| using ReadoutType = types::DUMMY_FRAME_STRUCT; | ||
|
|
||
| BOOST_AUTO_TEST_CASE(datahandlinglibs_DataHandlingModel_PostprocessScheduleAlgorithm_timeout) | ||
| { | ||
| std::atomic<bool> run_marker = true; | ||
|
|
||
| auto model = | ||
| unittest::MockDataHandlingModel<ReadoutType, | ||
| DefaultRequestHandlerModel<ReadoutType, SkipListLatencyBufferModel<ReadoutType>>, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want the same test with a queue?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No... delayed postprocessing only makes sense for skip list. |
||
| SkipListLatencyBufferModel<ReadoutType>, | ||
| TaskRawDataProcessorModel<ReadoutType>>(run_marker); | ||
|
|
||
| auto buffer = std::make_shared<SkipListLatencyBufferModel<ReadoutType>>(); | ||
|
|
||
| for (int i = 1; i < 6; i++) { | ||
| ReadoutType frame{}; | ||
| frame.timestamp = i * 62500; | ||
| buffer->write(std::move(frame)); | ||
| } | ||
|
|
||
| const bool post_processing_enabled = true; | ||
| auto error_registry = std::make_unique<FrameErrorRegistry>(); | ||
|
|
||
| auto raw_processor = | ||
| std::make_shared<TaskRawDataProcessorModel<ReadoutType>>(error_registry, post_processing_enabled); | ||
|
|
||
| const uint64_t delay_ticks = 4 * 62500; // NOLINT(build/unsigned) | ||
| const uint64_t delay_min_wait = 1; // NOLINT(build/unsigned) | ||
| const uint64_t delay_max_wait = 2; // NOLINT(build/unsigned) | ||
|
|
||
| typename decltype(model)::PostprocessScheduleAlgorithm sched_algo{ | ||
| *buffer, *raw_processor, delay_ticks, delay_min_wait, delay_max_wait | ||
| }; | ||
|
|
||
| // First pass | ||
| bool timeout = false; | ||
| int processed_count = sched_algo.run(timeout); | ||
| // Buffer = {1, 2, 3, 4, 5} delay_ticks = 4 | ||
| // 5 - 1 > 4 is false => no postprocessing | ||
| BOOST_REQUIRE_EQUAL(processed_count, 0); | ||
|
|
||
| timeout = true; | ||
| // 1st timeout => timeout_accumulated = 1 * 2 (delay_max_wait = 2) | ||
| // end_win_ts = 5 - 4 + 2 => postprocess until 3 {1, 2} | ||
| processed_count += sched_algo.run(timeout); | ||
| BOOST_REQUIRE_EQUAL(processed_count, 2); | ||
|
|
||
| // 2nd timeout => timeout_accumulated = 2 * 2 | ||
| // end_win_ts = 5 - 4 + 4 => postprocess until 5 {3, 4} | ||
| processed_count += sched_algo.run(timeout); | ||
| BOOST_REQUIRE_EQUAL(processed_count, 4); | ||
|
|
||
| // 3rd timeout => timeout_accumulated = 3 * 2 | ||
| // end_win_ts = 5 - 4 + 6 => postprocess until 6 (capped to newest_ts + 1) {5} | ||
| processed_count += sched_algo.run(timeout); | ||
| BOOST_REQUIRE_EQUAL(processed_count, 5); | ||
| } | ||
|
|
||
| BOOST_AUTO_TEST_SUITE_END() | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this is just an optimization, but maybe still worth considering:
After a few timeouts,
end_win_tswill settle onnewest_ts+1, so maybe one could check here ifend_win_ts >= m_unprocessed_element.get_timestamp()and if so, stop the processing here.@denizergonul let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest
m_processed_up_to.get_timestamp() >= newest_ts + 1There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(so we don't even calculate
end_win_ts)