-
Notifications
You must be signed in to change notification settings - Fork 138
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
Perf[MQB]: do not build temporary functors for every routed message #477
Conversation
Signed-off-by: Evgeny Malygin <[email protected]>
66be800
to
c6d38ea
Compare
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.
Build 331 of commit c6d38ea has completed with FAILURE
allocator, | ||
&QueueEngineUtil_AppsDeliveryContext::visitBroadcast, | ||
this, | ||
bdlf::PlaceHolders::_1)) |
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.
The alternative to this is to make Visitor
functor and inherit from it locally.
Like here:
#481
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.
To implement new DeliveryContext interface...
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.
Let's discuss it later then
allocator, | ||
&QueueEngineUtil_AppsDeliveryContext::visitBroadcast, | ||
this, | ||
bdlf::PlaceHolders::_1)) |
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.
To implement new DeliveryContext interface...
Should save 1% out of 36.4% of the queue dispatcher thread and reduce the total number of allocations.
Before:
After: