Skip to content

Use DecompressionStream when available#1971

Merged
FrederikBolding merged 8 commits intomainfrom
fb/npm-performance
Dec 4, 2023
Merged

Use DecompressionStream when available#1971
FrederikBolding merged 8 commits intomainfrom
fb/npm-performance

Conversation

@FrederikBolding
Copy link
Copy Markdown
Member

@FrederikBolding FrederikBolding commented Nov 16, 2023

This PR changes the NPM unpacking implementation slightly to use native browser DecompressionStream for gzip decompression, where possible. If not possible, we use browserify-zlib instead of gunzip-maybe as it is slightly faster.

Also adds a header to the NPM manifest fetching code that strips a bunch of unnecessary information.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a6ea785) 96.08% compared to head (a822b33) 96.08%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1971   +/-   ##
=======================================
  Coverage   96.08%   96.08%           
=======================================
  Files         267      267           
  Lines        6230     6240   +10     
  Branches     1007     1009    +2     
=======================================
+ Hits         5986     5996   +10     
  Misses        244      244           

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

@FrederikBolding FrederikBolding changed the title Experiment with NPM performance improvements Use DecompressionStream when available Dec 4, 2023
@FrederikBolding FrederikBolding marked this pull request as ready for review December 4, 2023 10:40
@FrederikBolding FrederikBolding requested a review from a team as a code owner December 4, 2023 10:40
@FrederikBolding FrederikBolding merged commit 0bbe326 into main Dec 4, 2023
@FrederikBolding FrederikBolding deleted the fb/npm-performance branch December 4, 2023 10:53
FrederikBolding added a commit that referenced this pull request Dec 19, 2023
Following the merge of #1971 we saw increased failures when fetching
snaps. We haven't been able to fully understand why yet, but it looks
like it may be a race condition in some of the stream code. Out of
caution, we revert part of the PR to stop using `DecompressionStream`
for now.
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.

2 participants