Implement scheduler timers using asio deadline timers #3056
Conversation
|
@cristofermartins some people do not understand the reason for some changes, can you explain what benefits does this change bring? Or is it just a more modern way of writing the same thing? |
|
@MillhioreBT just another way to implement timers. But also #2218 |
I already understand thank you very much, it is important for better precision in all sub-processes, I will definitely test it |
|
As a note, in my tests, there is min 4 max 74 microseconds delay to io_service execute the addevent/stopevent lambda in linux. |
DSpeichert
added
the
enhancement
| std::unordered_set<uint32_t> eventIds; | ||
| std::atomic<uint32_t> lastEventId {0}; | ||
| std::unordered_map<uint32_t, boost::asio::deadline_timer*> eventIdTimerMap; | ||
| std::vector<boost::asio::deadline_timer*> timerCacheVec; |
There was a problem hiding this comment.
Does it require pointers? I would store instances otherwise. Don't like the sight of two levels of indirections, you can probably save some extra time by avoinding that.
There was a problem hiding this comment.
I believe if i don't use pointers i can't reuse the deadline_timer object. Got ideas? "Don't like the sight of two levels of indirections" elaborate please.
There was a problem hiding this comment.
Vector is allocated on the heap, and if you use pointers for deadline_timers they will also be allocated in the heap, possibly incurring in performance issues if these are accessed often and out of order. If you use the objects instead of pointers, they are stored contiguouly, so you avoid one round trip to the slow RAM.
Why/when would you reuse the same object?
| // if not generate one | ||
| if (++lastEventId == 0) { | ||
| lastEventId = 1; | ||
| if (tmpEventId == 0) { |
There was a problem hiding this comment.
How would it ever be 0 if lastEventId is initialized to 0 and incremented before testing? If you fear overflow, just increase to 64 bit.
| if (it == eventIdTimerMap.end()) { | ||
| return; | ||
| } | ||
| it->second->cancel(); |
There was a problem hiding this comment.
You can invert the condition (it != end) to save a line here.
| io_service.post([this]() { | ||
| for (auto* timer : timerCacheVec) { | ||
| delete timer; | ||
| } |
There was a problem hiding this comment.
If you end up not using pointers, you can probably get off by replacing the loop with timerCacheVec = {}
|
@cristofermartins any update on this? This is one of the last prs marked for 1.4 |
| std::unordered_set<uint32_t> eventIds; | ||
| std::atomic<uint32_t> lastEventId {0}; | ||
| std::unordered_map<uint32_t, boost::asio::deadline_timer*> eventIdTimerMap; | ||
| std::vector<boost::asio::deadline_timer*> timerCacheVec; |
There was a problem hiding this comment.
I believe if i don't use pointers i can't reuse the deadline_timer object. Got ideas? "Don't like the sight of two levels of indirections" elaborate please.
| timer = timerCacheVec.back(); | ||
| timerCacheVec.pop_back(); | ||
| } | ||
| eventIdTimerMap[task->getEventId()] = timer; |
There was a problem hiding this comment.
I get when you are reusing the same timer objects here. Is the construction of a timer that heavy that you can't create a new one every time, and delete it when done?
|
Sorry, misclick |
@cristofermartins |
|
Just wanted to add that boost::asio::io_service is deprecated, more info: https://stackoverflow.com/questions/59753391/boost-asio-io-service-vs-io-context |
As a bonus someone can use scheduler thread to run tasks using the io_service.post function.