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.

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

Remove most locking overhead in TripleBuffer #5915

Merged
merged 3 commits into from
Jul 24, 2023

Conversation

peppy
Copy link
Member

@peppy peppy commented Jul 10, 2023

One of those things I see in passing and can't un-see. Call this a developer indulgence rather than an optimisation if you want.

Before After
JetBrains Rider 2021-09 at 07 04 10 JetBrains Rider 2023-07-10 at 07 09 26
JetBrains Rider 2021-09 at 07 03 40 JetBrains Rider 2023-07-10 at 07 09 00

The time profiling results should be taken with a grain of salt. Of note is the reduction in lock %, mainly in finishUsage.

It's such an edge case it's not going to improve anything anyway.
@pull-request-size pull-request-size bot added size/M and removed size/L labels Jul 10, 2023
@bdach
Copy link
Collaborator

bdach commented Jul 10, 2023

Can't see any fault with this, theoretically speaking. The split critical section in getPendingReadBuffer() (the xchg first and the monitor block second) had me worried the most and even then I don't see how it can be abused to do anything transgressive.

Will probably ask for a second review though...

@bdach bdach requested a review from smoogipoo July 10, 2023 18:38
@peppy peppy merged commit ddc2488 into ppy:master Jul 24, 2023
@peppy
Copy link
Member Author

peppy commented Jul 24, 2023

(checked with smoogi and he was fine with getting this in)

@peppy peppy deleted the remove-triple-buffer-locking branch September 14, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants