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

Reduce allocations and branching in transform_code_lengths_to_code #27

Merged

Conversation

LilithHafner
Copy link
Contributor

This is a small performance improvement. It is not visible when running end to end benchmarks (nor is there a visible regression). We can see an improvement when just benchmarking the target function, though:

julia> Random.seed!(13); for i in 1:10
           x = rand(1:i, rand(1:i))
           display(@b x Inflate.transform_code_lengths_to_code)
       end
61.461 ns (4 allocs: 272 bytes)
98.814 ns (6 allocs: 416 bytes)
137.207 ns (8 allocs: 560 bytes)
105.065 ns (6 allocs: 400 bytes)
191.794 ns (11 allocs: 768 bytes)
207.341 ns (11 allocs: 768 bytes)
164.368 ns (9 allocs: 592 bytes)
261.114 ns (14 allocs: 960 bytes)
295.660 ns (16 allocs: 1.328 KiB)
304.303 ns (16 allocs: 1.094 KiB)

shell> git stash pop
...

julia> Random.seed!(13); for i in 1:10
           x = rand(1:i, rand(1:i))
           display(@b x Inflate.transform_code_lengths_to_code)
       end
46.199 ns (3 allocs: 208 bytes)
81.508 ns (5 allocs: 352 bytes)
113.339 ns (7 allocs: 512 bytes)
86.581 ns (5 allocs: 352 bytes)
163.136 ns (10 allocs: 736 bytes)
171.270 ns (10 allocs: 736 bytes)
131.980 ns (8 allocs: 560 bytes)
220.342 ns (13 allocs: 944 bytes)
223.109 ns (14 allocs: 1.000 KiB)
254.167 ns (15 allocs: 1.078 KiB)

Looping through the input a second time is cheap compared to the additional branching and allocations within the loop. IMO this is also slightly simpler to understand, though the previous version was also quite legible.

@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.35%. Comparing base (f16eec8) to head (166c274).
Report is 6 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
- Coverage   99.35%   99.35%   -0.01%     
==========================================
  Files           1        1              
  Lines         468      467       -1     
==========================================
- Hits          465      464       -1     
  Misses          3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GunnarFarneback
Copy link
Owner

Looks fine, thanks.

@GunnarFarneback GunnarFarneback merged commit a143c73 into GunnarFarneback:master Sep 24, 2024
16 of 17 checks passed
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