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

Remove unnecessary unaliasing allocation #24

Merged

Conversation

LilithHafner
Copy link
Contributor

Profiling reveals that this append! call allocates. Benchmarking reveals that changing the implementation speeds things up. Hopefully future versions of Julia will be clever enough to determine that there is no aliasing in the append! case but on Julia 1.10.5 append! makes a copy to prevent possible aliasing.

PR In Memory
incompressible huffman runlength graph
small deflate 2.0 2.9 4.0 2.4
zlib 2.7 2.9 3.9 2.5
gzip 3.9 3.0 3.4 2.6
medium deflate 1.5 3.3 2.6 3.1
zlib 3.6 3.5 2.8 3.2
gzip 5.0 3.5 3.5 3.4
large deflate 3.5 3.5 2.6 3.3
zlib 4.1 3.5 2.8 3.3
gzip 5.2 3.6 3.2 3.6
master In Memory
incompressible huffman runlength graph
small deflate 2.1 3.4 3.9 3.5
zlib 2.8 3.5 3.9 3.6
gzip 4.2 3.6 3.5 3.5
medium deflate 1.6 4.4 3.8 5.1
zlib 3.5 4.5 4.0 4.8
gzip 4.9 4.9 3.9 4.8
large deflate 3.5 4.7 3.8 7.0
zlib 4.0 4.6 3.8 5.1
gzip 5.4 4.7 4.3 5.2

This is a big improvement for the large deflate graph case!

julia> @b compressed inflate
129.791 ms (1605215 allocs: 118.158 MiB, 2.13% gc time)

shell> git checkout lh/perf-alloc-1
Previous HEAD position was cc77be7 Fix correctness for 32-bit Julia.
Switched to branch 'lh/perf-alloc-1'
Your branch is up to date with 'origin/lh/perf-alloc-1'.

julia> @b compressed inflate
81.500 ms (8300 allocs: 20.690 MiB)

@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.36%. Comparing base (f16eec8) to head (4e4f400).
Report is 4 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      #24   +/-   ##
=======================================
  Coverage   99.35%   99.36%           
=======================================
  Files           1        1           
  Lines         468      470    +2     
=======================================
+ Hits          465      467    +2     
  Misses          3        3           

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

@GunnarFarneback
Copy link
Owner

Looks good. Notice that this was one of the optimizations identified in #9, but either this implementation is better or it has become more effective since then.

@GunnarFarneback GunnarFarneback merged commit a73ab50 into GunnarFarneback:master Sep 23, 2024
17 checks passed
@LilithHafner LilithHafner deleted the lh/perf-alloc-1 branch September 23, 2024 20:04
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