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

Porting IOQueue lock free implementation #7449

Merged
merged 1 commit into from
Dec 22, 2021

Conversation

brunolins16
Copy link
Member

@brunolins16 brunolins16 commented Dec 21, 2021

Fixes #7392

Microsoft Reviewers: Open in CodeFlow

@ReubenBond ReubenBond self-assigned this Dec 21, 2021
@brunolins16
Copy link
Member Author

@ReubenBond, do you know which test or benchmark I can run to verify the changes? It seems working just fine, but I would like to check if there are no regressions.

@ReubenBond
Copy link
Member

ReubenBond commented Dec 21, 2021

This might be a good candidate for the new Crank-based distributed tests which Ben added, but he is away and I'm not sure how best to run them yet. Alternatively, there's the ConcurrentPing tests which are a part of the benchmarks app in the test folder. There's a script you can use to launch it: https://github.com/dotnet/orleans/blob/main/test/Benchmarks/run_test.cmd

@brunolins16
Copy link
Member Author

This might be a good candidate for the new Crank-based distributed tests which Ben added, but he is away and I'm not sure how best to run them yet. Alternatively, there's the ConcurrentPing tests which are a part of the benchmarks app in the test folder. There's a script you can use to launch it: https://github.com/dotnet/orleans/blob/main/test/Benchmarks/run_test.cmd

I ran the ConcurrentPing tests and also included the ThreadingDiagnoser and they results are very similar, of course there are too much noise in my machine. The only thing I noticed is the number of Lock contentions is a little bit higher with the new code, in every test I ran, maybe because the number of Completed Work Items is higher.

Before

Method Mean Error StdDev Gen 0 Completed Work Items Lock Contentions Gen 1 Gen 2 Allocated
Ping 144.6 s 9.61 s 1.49 s 856000.0000 46769547.0000 25634.0000 83000.0000 27000.0000 29 GB

After

Method Mean Error StdDev Gen 0 Completed Work Items Lock Contentions Gen 1 Gen 2 Allocated
Ping 136.8 s 177.9 s 9.75 s 870000.0000 47166465.0000 27976.0000 85000.0000 26000.0000 29 GB

@ReubenBond
Copy link
Member

This is about what I would expect, @brunolins16. It would be hard to detect a change here without a specifically targeted microbenchmark. That's fine - we can trust that the upstream change was made for a good reason and with appropriate testing

@ReubenBond ReubenBond merged commit eaf9ae1 into dotnet:main Dec 22, 2021
@brunolins16 brunolins16 deleted the brunolins16/issues/7392 branch December 22, 2021 17:46
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

This latest lock free implementation can be applied to Orleans
2 participants