Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve the performance of expireTimeouts() in HashedWheelTimer #12888

Merged
merged 1 commit into from
Oct 14, 2022
Merged

Improve the performance of expireTimeouts() in HashedWheelTimer #12888

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions common/src/main/java/io/netty5/util/HashedWheelTimer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public class HashedWheelTimer implements Timer {
private final AtomicLong pendingTimeouts = new AtomicLong(0);
private final long maxPendingTimeouts;
private final Executor taskExecutor;
private long currRound;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering, shouldn't this field be moved to Worker class where it belongs? (otherwise it is potentially shared between multiple threads and may need volatile, seems to be unnecessary)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a logic error in my code. Please refer to #13018 for detail.


private volatile long startTime;

Expand Down Expand Up @@ -495,11 +496,14 @@ public void run() {
final long deadline = waitForNextTick();
if (deadline > 0) {
int idx = (int) (tick & mask);
if (idx == 0 && tick > 0) {
currRound ++;
}
processCancelledTasks();
HashedWheelBucket bucket =
wheel[idx];
transferTimeoutsToBuckets();
bucket.expireTimeouts(deadline);
bucket.expireTimeouts(deadline, currRound);
tick++;
}
} while (WORKER_STATE_UPDATER.get(HashedWheelTimer.this) == WORKER_STATE_STARTED);
Expand Down Expand Up @@ -535,7 +539,7 @@ private void transferTimeoutsToBuckets() {
}

long calculated = timeout.deadline / tickDuration;
timeout.remainingRounds = (calculated - tick) / wheel.length;
timeout.execRound = calculated / wheel.length;

final long ticks = Math.max(calculated, tick); // Ensure we don't schedule for past.
int stopIndex = (int) (ticks & mask);
Expand Down Expand Up @@ -625,9 +629,9 @@ private static final class HashedWheelTimeout implements Timeout, Runnable {
@SuppressWarnings({"unused", "FieldMayBeFinal", "RedundantFieldInitialization" })
private volatile int state = ST_INIT;

// remainingRounds will be calculated and set by Worker.transferTimeoutsToBuckets() before the
// execRound will be calculated and set by Worker.transferTimeoutsToBuckets() before the
// HashedWheelTimeout will be added to the correct HashedWheelBucket.
long remainingRounds;
long execRound;

// This will be used to chain timeouts in HashedWheelTimerBucket via a double-linked-list.
// As only the workerThread will act on it there is no need for synchronization / volatile.
Expand Down Expand Up @@ -777,13 +781,13 @@ public void addTimeout(HashedWheelTimeout timeout) {
/**
* Expire all {@link HashedWheelTimeout}s for the given {@code deadline}.
*/
public void expireTimeouts(long deadline) {
public void expireTimeouts(long deadline, long currRound) {
HashedWheelTimeout timeout = head;

// process all timeouts
while (timeout != null) {
HashedWheelTimeout next = timeout.next;
if (timeout.remainingRounds <= 0) {
if (timeout.execRound <= currRound) {
next = remove(timeout);
if (timeout.deadline <= deadline) {
timeout.expire();
Expand All @@ -795,7 +799,7 @@ public void expireTimeouts(long deadline) {
} else if (timeout.isCancelled()) {
next = remove(timeout);
} else {
timeout.remainingRounds --;
break;
}
timeout = next;
}
Expand Down