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

lock-free map bench #181

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

anmolsahoo25
Copy link
Contributor

This patch implements a lock-free map using tries. The design is inspired by this paper - Link.

This benchmark is a decent mix between domains performing work, synchronization via CAS and garbage collection. Preliminary results on my laptop seemed to produce speedups, but testing on IITM machines is producing a slowdown, as cores are added. Debugging before merge.

@anmolsahoo25
Copy link
Contributor Author

Ah, made a mistake in benchmarking. Was making each domain perform a fixed number of operations. Rather, need to split fixed number of operations between domains. Some results -

Duration
index

Speedup
index2

@anmolsahoo25 anmolsahoo25 changed the title [Dont-Merge] WIP for lock-free map WIP for lock-free map Oct 6, 2020
@anmolsahoo25 anmolsahoo25 changed the title WIP for lock-free map lock-free map bench Oct 6, 2020
@anmolsahoo25
Copy link
Contributor Author

After implementing DLS, results look much better. Still some pathology around 20 cores though.
index
index2

Copy link
Member

@ctk21 ctk21 left a comment

Choose a reason for hiding this comment

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

I haven't spent quality time reviewing the underlying data structure (which looks interesting and worthwhile).

However there are a couple of problems with the work distribution; these could be easily fixed by using domainslib's parallel_for.

I would also recommend increasing the benchmark size; looking at scaling up above 16 domains when the runtime is already 0.7s might get quite noise.

I'd be interested if that improves the scaling you see.

@@ -0,0 +1,27 @@
let threads = int_of_string Sys.argv.(1)
let insert_percent = int_of_string Sys.argv.(2)
let num_opers = int_of_string Sys.argv.(3) / threads
Copy link
Member

Choose a reason for hiding this comment

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

There will be settings of Sys.argv.(3) and threads where the total number of operations is not the same between runs.
e.g. Sys.argv.(3) = 100 and threads=128

Better to use parallel_for in domainslib.

| 0 -> []
| _ -> (Domain.spawn work) :: spawn_thread (n-1)
in
let threads = spawn_thread (threads - 1) in
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to use domainslib parallel_for if you want scaling beyond 16-32 cores.

@anmolsahoo25
Copy link
Contributor Author

Thank you for the review. I will get to it this weekend and resolve the issues, so that we can hopefully add it to the sandmark testsuite.

@Sudha247
Copy link
Contributor

Sudha247 commented Apr 7, 2022

Hey @anmolsahoo25, lockfree might be a good place for the lockfree map. Feel free to submit a patch!

@anmolsahoo25
Copy link
Contributor Author

Thanks @Sudha247 , will do! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants