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

Replace get! with get to avoid unnecessary initialization of state values #594

Conversation

bowenszhu
Copy link
Member

The original code used get! which would initialize state[t] to 0 if it didn't exist before incrementing. This is unnecessary as the value is only used for incrementing. Using get with a default value of 0 achieves the same result without potential performance overhead.

See the source code for get:
https://github.com/JuliaLang/julia/blob/0b4590a5507d3f3046e5bafc007cacbbfc9b310b/base/dict.jl#L545
and for get!:
https://github.com/JuliaLang/julia/blob/0b4590a5507d3f3046e5bafc007cacbbfc9b310b/base/dict.jl#L473

Copy link
Contributor

github-actions bot commented May 6, 2024

Benchmark Results

master 5320dd7... master/5320dd74f75ffc...
overhead/acrule/a+2 0.753 ± 0.021 μs 0.725 ± 0.021 μs 1.04
overhead/acrule/a+2+b 0.746 ± 0.019 μs 0.721 ± 0.021 μs 1.04
overhead/acrule/a+b 0.254 ± 0.012 μs 0.262 ± 0.013 μs 0.97
overhead/acrule/noop:Int 25.4 ± 0.92 ns 26.2 ± 0.93 ns 0.967
overhead/acrule/noop:Sym 0.0334 ± 0.0059 μs 0.0342 ± 0.0051 μs 0.976
overhead/rule/noop:Int 0.0379 ± 0.00043 μs 0.0376 ± 0.001 μs 1.01
overhead/rule/noop:Sym 0.0439 ± 0.0015 μs 0.0424 ± 0.0013 μs 1.04
overhead/rule/noop:Term 0.0435 ± 0.0016 μs 0.0428 ± 0.0018 μs 1.02
overhead/ruleset/noop:Int 0.127 ± 0.0036 μs 0.127 ± 0.0021 μs 1
overhead/ruleset/noop:Sym 0.133 ± 0.0037 μs 0.131 ± 0.0028 μs 1.01
overhead/ruleset/noop:Term 6.75 ± 0.54 μs 6.59 ± 0.42 μs 1.02
overhead/simplify/noop:Int 0.141 ± 0.0033 μs 0.14 ± 0.0027 μs 1.01
overhead/simplify/noop:Sym 0.151 ± 0.0038 μs 0.154 ± 0.0039 μs 0.981
overhead/simplify/noop:Term 0.0444 ± 0.0023 ms 0.045 ± 0.0027 ms 0.988
overhead/simplify/randterm (+, *):serial 0.133 ± 0.0032 s 0.135 ± 0.0019 s 0.984
overhead/simplify/randterm (+, *):thread 0.0868 ± 0.027 s 0.0842 ± 0.028 s 1.03
overhead/simplify/randterm (/, *):serial 0.272 ± 0.0088 ms 0.279 ± 0.0093 ms 0.973
overhead/simplify/randterm (/, *):thread 0.324 ± 0.01 ms 0.334 ± 0.011 ms 0.97
overhead/substitute/a 0.121 ± 0.0028 ms 0.122 ± 0.0031 ms 0.992
overhead/substitute/a,b 0.0995 ± 0.0027 ms 0.103 ± 0.0027 ms 0.966
overhead/substitute/a,b,c 16.7 ± 0.69 μs 19.5 ± 0.86 μs 0.856
polyform/easy_iszero 0.053 ± 0.0025 ms 0.0527 ± 0.0021 ms 1
polyform/isone 2.79 ± 0.01 ns 2.79 ± 0.01 ns 1
polyform/iszero 5.57 ± 0.1 ms 5.54 ± 0.081 ms 1
polyform/simplify_fractions 3.38 ± 0.065 ms 3.39 ± 0.048 ms 0.997
time_to_load 4.56 ± 0.048 s 4.57 ± 0.0037 s 0.997

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@shashi shashi merged commit cc5eba3 into master May 7, 2024
6 of 11 checks passed
@shashi shashi deleted the b/replace_get!_with_get_to_avoid_unnecessary_initialization_of_state_values branch May 7, 2024 19:32
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