-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
feat: port node:zlib to rust #18291
feat: port node:zlib to rust #18291
Conversation
Some improvement in memory usage while there is no visible difference in startup time. |
This PR:
So we got 270kB saved here. That's a huge improvement 👍 EDIT: Looking at flamegraphs I can see about 2 point reduction in time it takes to deserialize the snapshot and create a context (~59% -> ~57%) |
0916664
to
917ab1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far.. some early comments.
const GZIP_HEADER_ID2: u8 = 0x8b; | ||
|
||
impl ZlibInner { | ||
fn start_write( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize the zlib API is not terribly ergonomic, but bifurcating the compress/decompress paths into separate functions may simplify the code. I don't think this necessarily has to happen, but it would be much easier to understand if the code paths where separated in that way.
~1ms exec time improvement on Linux
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one small nit
Co-authored-by: Matt Mastracci <[email protected]>
Following a more simpler approach than #18176.
TODO: