Skip to content

Conversation

normanmaurer
Copy link
Member

Motivation:

When calling Selector.whenReady(...) and using a SelectorStrategy.blockUntil(...) we may end up re-arm the timerfd each time even if the wakeup time did not change.
This is expensive and can be avoided in most situations.

Modifications:

Keep track of the earliest scheduled timer and only schedule a new one if it would fire sooner.

Result:

Less overhead when using epoll and timers.

@normanmaurer
Copy link
Member Author

I need to write a few tests but the idea is the same as:

netty/netty#7816

@normanmaurer normanmaurer force-pushed the timerfd_rearm branch 2 times, most recently from 0205ccd to 7ae80fe Compare March 30, 2018 19:17
@normanmaurer normanmaurer requested review from Lukasa and weissi March 30, 2018 19:18
@normanmaurer
Copy link
Member Author

Test added PTAL

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

LGTM, I'm happy with this.

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Apr 3, 2018
@Lukasa Lukasa added this to the 1.4.0 milestone Apr 3, 2018
// expensive.
let next = DispatchTime.now().uptimeNanoseconds + UInt64(timeAmount.nanoseconds)
if next < earliestTimer {
earliestTimer = next
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, please use explicit self's on earliestTimer`. It'll help make this a lot clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Motivation:

When calling Selector.whenReady(...) and using a SelectorStrategy.blockUntil(...) we may end up re-arm the timerfd each time even if the wakeup time did not change.
This is expensive and can be avoided in most situations.

Modifications:

Keep track of the earliest scheduled timer and only schedule a new one if it would fire sooner.

Result:

Less overhead when using epoll and timers.
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Cool, I'm happy. Will wait for @weissi to review.

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

awesome, lgtm!

@weissi weissi merged commit 2a684fc into apple:master Apr 3, 2018
@normanmaurer normanmaurer deleted the timerfd_rearm branch April 23, 2018 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants