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

Better stable branch hints #358

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Conversation

aumetra
Copy link
Contributor

@aumetra aumetra commented Nov 10, 2023

(This is a relatively small change, hence no prior discussion in an issue)

This PR adapts the technique outlined in hashbrown for branch hints on stable, utilizing the #[cold] attribute.
In my local benchmarks I observed an improvement of ~8-13% on average (but I'm not quite sure how many of these were flukes).

Copy link
Member

@Licenser Licenser left a comment

Choose a reason for hiding this comment

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

This looks really cool :) thanks for sharing it and contributing it!

@Licenser
Copy link
Member

The differences don't seem to be in the 8% range but they are measurable through the benchmarks :D well done 👍

group                                                    base                                   changes
-----                                                    ----                                   -------
apache_builds/simd_json::to_tape_with_buffers            1.01     90.7±4.85µs  1337.7 MB/sec    1.00     90.2±2.78µs  1345.7 MB/sec
canada/simd_json::to_tape_with_buffers                   1.00      3.6±0.14ms   601.4 MB/sec    1.00      3.6±0.18ms   599.0 MB/sec
citm_catalog/simd_json::to_tape_with_buffers             1.01  1115.1±81.18µs  1477.2 MB/sec    1.00  1109.5±82.41µs  1484.7 MB/sec
event_stacktrace_10kb/simd_json::to_tape_with_buffers    1.01      3.7±0.29µs     2.5 GB/sec    1.00      3.7±0.23µs     2.5 GB/sec
github_events/simd_json::to_tape_with_buffers            1.00     40.0±1.52µs  1553.9 MB/sec    1.00     39.9±2.04µs  1556.9 MB/sec
log/simd_json::to_tape_with_buffers                      1.00  1551.0±85.81ns  1337.4 MB/sec    1.00  1557.2±104.51ns  1332.0 MB/sec
twitter/simd_json::to_tape_with_buffers                  1.01   437.4±27.53µs  1376.8 MB/sec    1.00   434.6±28.27µs  1385.7 MB/sec
Warning: Failed to comment: HttpError: Resource not accessible by integration
Commenting is not possible from forks.
┌─────────┬─────────────────────────────────────────────────────────┬──────────────────┬──────────────────────┬────────────┐
│ (index) │                          name                           │   baseDuration   │   changesDuration    │ difference │
├─────────┼─────────────────────────────────────────────────────────┼──────────────────┼──────────────────────┼────────────┤
│    0    │     'apache_builds/simd_json::to_tape_with_buffers'     │  '90.7±4.85µs'   │  '**90.2±2.78µs**'   │  '-0.99'   │
│    1    │        'canada/simd_json::to_tape_with_buffers'         │   '3.6±0.14ms'   │     '3.6±0.18ms'     │   '0.0'    │
│    2    │     'citm_catalog/simd_json::to_tape_with_buffers'      │ '1115.1±81.18µs' │ '**1109.5±82.41µs**' │  '-0.99'   │
│    3    │ 'event_stacktrace_10kb/simd_json::to_tape_with_buffers' │   '3.7±0.29µs'   │   '**3.7±0.23µs**'   │  '-0.99'   │
│    4    │     'github_events/simd_json::to_tape_with_buffers'     │  '40.0±1.52µs'   │    '39.9±2.04µs'     │   '0.0'    │
│    5    │          'log/simd_json::to_tape_with_buffers'          │ '1551.0±85.81ns' │  '1557.2±104.51ns'   │   '0.0'    │
│    6    │        'twitter/simd_json::to_tape_with_buffers'        │ '437.4±27.53µs'  │ '**434.6±28.27µs**'  │  '-0.99'   │
│    7    │                           ''                            │    undefined     │      undefined       │   '+NaN'   │
└─────────┴─────────────────────────────────────────────────────────┴──────────────────┴──────────────────────┴────────────┘

@Licenser Licenser merged commit c83335a into simd-lite:main Nov 10, 2023
23 checks passed
@aumetra aumetra deleted the stable-branch-hints branch November 10, 2023 11:26
@aumetra
Copy link
Contributor Author

aumetra commented Nov 10, 2023

Heh, I guess the 8% were just my laptop acting up a bit then

@Licenser
Copy link
Member

:D no harm it's an improvement one way or the other, doesn't always need to be 8%, this gives 1% in the tests, and might very well do more in other situations. What matters is that it's better in all situations so it's not a fluke but a through the board improvement 🚀

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.

2 participants