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

Merge into standard library #3

Closed
mholt opened this issue Jul 28, 2015 · 13 comments
Closed

Merge into standard library #3

mholt opened this issue Jul 28, 2015 · 13 comments

Comments

@mholt
Copy link

mholt commented Jul 28, 2015

Good work with these optimizations! Is there a reason that these improvements shouldn't/wouldn't be merged into the standard library?

@klauspost
Copy link
Owner

It is fairly specific to x64 - which IS the main majority I would expect). Without the assembler code, the changes aren't that big, and might be worse on 32 bit systems.

But after a while I might submit it, I would like for it to be field-tested a bit before that.

@mholt
Copy link
Author

mholt commented Jul 28, 2015

Fair enough. I think the improvements are worth the effort. I'll try to deploy this package in an app and will let you know how it fares. Thanks for sharing!

@eaigner
Copy link

eaigner commented Jul 28, 2015

Nowadays most machines are x64, so I guess the core lib would definitely benefit from it.

@shishkander
Copy link

For our workloads (~2GB executables uploaded with gzipping), your package gives about 2x speed gain in my benchmarks on 64bit Linux.

@gorakhargosh
Copy link

+1.

Great work!

I'd request you to sent these for code reviews one by one instead
of waiting until all of it is ready. Very large code changes tend to be
hard to review and require setting time apart just for reviewing.

(Have experienced this first-hand on internal CLs.)

Thank you. :)

Cheers,
Yesudeep.

@ghost
Copy link

ghost commented Aug 26, 2015

Nice work. I've use this successfully in one of our projects.
It would great to get it into the standard library too. Let me know if there is anything I can do to help with that somehow.

@mholt
Copy link
Author

mholt commented Oct 10, 2015

Just reporting back -- been using this in production for several months now with 0 (known) problems.

@derekperkins
Copy link

We're using this and haven't had any issues. Is it too late for 1.6?

@klauspost
Copy link
Owner

1.6 is closed for new features (and has been for >1 month).

My aim is to submit it at the start of 1.7, so it can live in "tip" for a while.

My aim is to weed out subtle and rare failures like #20, which has gone undetected since the start of the project.

@klauspost
Copy link
Owner

It has begun: https://go-review.googlesource.com/19336

@mholt
Copy link
Author

mholt commented Apr 6, 2016

@klauspost Very nice! So this was merged and it looks like Go 1.6.1 comes out very soon. Do you know if your contributions will be in that release? Is this repo ahead of those contributions at all? (Looks like a few commits squeezed in here after being merged into golang/go.)

@nhooyr
Copy link

nhooyr commented May 27, 2016

Have all of these changes been merged into the standard library?

@klauspost
Copy link
Owner

klauspost commented Aug 20, 2016

Some of it has been merged into Go 1.7, closing the gap to ~10%. The Go deflate has not been rebalanced, so using "standard" is still ~1.5x faster, but loses ~2% in efficiency.

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

No branches or pull requests

7 participants