search

ulikunitz / xz

Pure golang package for reading and writing xz-compressed files
Other
484 stars 45 forks source link

lzma: make operation a concrete type #57

Open nigeltao opened 1 year ago

nigeltao commented 1 year ago

Prior to this commit, operation was an interface type, which meant that every call to methods like binTree.NextOp or hashTable.NextOp, which could return either a match op or a literal op, would dynamically allocate memory (and eventually require garbage collection).

After this commit, the "package xz" benchmarks show a modest speed gain and a dramatic fall in the number of allocations.

name old speed new speed delta Reader-8 28.9MB/s ± 0% 32.0MB/s ± 0% +11.01% (p=0.008 n=5+5) Writer-8 6.50MB/s ± 0% 6.95MB/s ± 1% +6.99% (p=0.008 n=5+5)

name old alloc/op new alloc/op delta Reader-8 37.3MB ± 0% 15.5MB ± 0% -58.39% (p=0.008 n=5+5) Writer-8 80.2MB ± 0% 60.8MB ± 0% -24.19% (p=0.000 n=5+4)

name old allocs/op new allocs/op delta Reader-8 1.21M ± 0% 0.00M ± 0% -99.96% (p=0.008 n=5+5) Writer-8 1.22M ± 0% 0.00M ± 0% -99.62% (p=0.000 n=5+4)

ulikunitz commented 1 year ago

Hi NIgel, thank you for the commit. Would it be possible for you to adapt the change for the rewrite branch? The new branch supports multithreading and has a lot of other performance improvements.

nigeltao commented 1 year ago

Ah, I didn't know about the rewrite branch. Looking at it now, that branch has deleted lzma/operation.go (and uses an lz.Seq instead, from github.com/ulikunitz/lz). I don't think this PR would apply to the rewrite branch.

I don't know what the release schedule is for the rewrite branch, so I'll leave it up to you whether you still want it for the master branch.