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
Avoid use of global AtomicLong for ScheduledFutureTask ids #9599
Conversation
Motivation Currently a static AtomicLong is used to allocate a unique id whenever a task is scheduled to any event loop. This could be a source of contention if delayed tasks are scheduled at a high frequency and can be easily avoided by having a non-volatile id counter per queue. Modifications - Replace static AtomicLong ScheduledFutureTask#nextTaskId with a long field in AbstractScheduledExecutorService - Set ScheduledFutureTask#id based on this when adding the task to the queue (in event loop) instead of at construction time - Add simple benchmark Result Less contention / cache-miss possibility when scheduling future tasks Before: Benchmark (num) Mode Cnt Score Error Units scheduleLots 100000 thrpt 20 346.008 ± 21.931 ops/s Benchmark (num) Mode Cnt Score Error Units scheduleLots 100000 thrpt 20 654.824 ± 22.064 ops/s
Can one of the admins verify this patch? |
@netty-bot test this please |
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.
Good catch!!
@njhill good one! |
I need to think about how to port this to master in a safe way. For now I created a label "needs_backport" that we can use to mark PR's that need to be ported. Once done we should remove the label |
@normanmaurer great idea re the label. One nit tho, wouldn't "forward-port" be more accurate? :) |
Motivation
Currently the same static
AtomicLong
is used to allocate a unique id whenever a task is scheduled to any event loop. This could be a source of contention if delayed tasks are scheduled at a high frequency and can be easily avoided by having a non-volatile id counter per queue.Modifications
static AtomicLong ScheduledFutureTask#nextTaskId
with along
field inAbstractScheduledExecutorService
ScheduledFutureTask#id
based on this when adding the task to the queue (in event loop) instead of at construction timeResult
Less contention / cache-miss possibility when scheduling future tasks
Before:
After: