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

Addressing comments in Flurry #1

Merged
merged 8 commits into from
Nov 18, 2022
Merged

Addressing comments in Flurry #1

merged 8 commits into from
Nov 18, 2022

Conversation

JackThomson2
Copy link
Owner

@JackThomson2 JackThomson2 commented Nov 11, 2022

This is to address comments made in this PR: jonhoo/flurry#109

Key points:

  • Removed the option for nightly
  • No longer using the unsafe cell.
  • Aligning the cells the the size of a cach-line to help with cache invalidation

Some notes: I did look at using the thread_id but this is a nightly only API so I'll leave it out for now as we're fully working on stable

I'm seeing some suspiciously good benchmark results:

atomic_counter/2        time:   [266.96 us 271.03 us 274.81 us]
                        thrpt:  [119.24 Melem/s 120.90 Melem/s 122.74 Melem/s]

atomic_counter/4        time:   [337.74 us 339.58 us 341.31 us]
                        thrpt:  [96.008 Melem/s 96.497 Melem/s 97.021 Melem/s]

atomic_counter/8        time:   [355.62 us 358.51 us 361.44 us]
                        thrpt:  [90.660 Melem/s 91.400 Melem/s 92.142 Melem/s]

atomic_counter/16       time:   [447.33 us 450.47 us 453.74 us]
                        thrpt:  [72.217 Melem/s 72.742 Melem/s 73.252 Melem/s]

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

fast_counter_stable/2   time:   [26.046 us 26.210 us 26.363 us]
                        thrpt:  [1.2430 Gelem/s 1.2502 Gelem/s 1.2581 Gelem/s]

fast_counter_stable/4   time:   [16.023 us 16.117 us 16.219 us]
                        thrpt:  [2.0204 Gelem/s 2.0331 Gelem/s 2.0451 Gelem/s]

fast_counter_stable/8   time:   [24.720 us 25.833 us 26.912 us]
                        thrpt:  [1.2176 Gelem/s 1.2685 Gelem/s 1.3256 Gelem/s]

fast_counter_stable/16  time:   [50.007 us 50.833 us 51.691 us]
                        thrpt:  [633.92 Melem/s 644.62 Melem/s 655.27 Melem/s]

However it looked like we were actually being bottlenecked by the allocation time, when I increase the iter count in the benchmark we can see great scaling: at 16 core we're seeing a near 60x improvment over regular atomic numbers

atomic_counter/1        time:   [1.5215 ms 1.5311 ms 1.5414 ms]
                        thrpt:  [680.28 Melem/s 684.86 Melem/s 689.19 Melem/s]

atomic_counter/2        time:   [8.1665 ms 8.3089 ms 8.4518 ms]
                        thrpt:  [124.07 Melem/s 126.20 Melem/s 128.40 Melem/s]

atomic_counter/4        time:   [10.192 ms 10.250 ms 10.303 ms]
                        thrpt:  [101.77 Melem/s 102.30 Melem/s 102.88 Melem/s]

atomic_counter/8        time:   [10.021 ms 10.131 ms 10.234 ms]
                        thrpt:  [102.46 Melem/s 103.50 Melem/s 104.64 Melem/s]

atomic_counter/16       time:   [12.362 ms 12.393 ms 12.425 ms]
                        thrpt:  [84.390 Melem/s 84.610 Melem/s 84.823 Melem/s]

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

fast_counter_stable/1   time:   [1.5678 ms 1.5795 ms 1.5913 ms]
                        thrpt:  [658.92 Melem/s 663.88 Melem/s 668.81 Melem/s]

fast_counter_stable/2   time:   [820.95 us 824.81 us 828.56 us]
                        thrpt:  [1.2655 Gelem/s 1.2713 Gelem/s 1.2773 Gelem/s]

fast_counter_stable/4   time:   [429.10 us 430.62 us 432.17 us]
                        thrpt:  [2.4263 Gelem/s 2.4350 Gelem/s 2.4437 Gelem/s]

fast_counter_stable/8   time:   [240.59 us 242.92 us 245.44 us]
                        thrpt:  [4.2723 Gelem/s 4.3165 Gelem/s 4.3583 Gelem/s]

fast_counter_stable/16  time:   [206.01 us 210.24 us 214.74 us]
                        thrpt:  [4.8829 Gelem/s 4.9875 Gelem/s 5.0900 Gelem/s]

This was referenced Nov 15, 2022
@JackThomson2 JackThomson2 merged commit 38fcd51 into master Nov 18, 2022
@JackThomson2 JackThomson2 deleted the tidy-up branch November 18, 2022 13:52
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.

1 participant