Skip to content

perf: improve tarball fetching speed#6819

Merged
zkochan merged 5 commits intomainfrom
fetch-perf
Aug 8, 2023
Merged

perf: improve tarball fetching speed#6819
zkochan merged 5 commits intomainfrom
fetch-perf

Conversation

@zkochan
Copy link
Copy Markdown
Member

@zkochan zkochan commented Jul 17, 2023

Comment thread fetching/tarball-fetcher/src/remoteTarballFetcher.ts Outdated
Comment thread fetching/tarball-fetcher/src/remoteTarballFetcher.ts Outdated
@zkochan zkochan force-pushed the fetch-perf branch 2 times, most recently from 27e1818 to 1575bd2 Compare July 24, 2023 00:22
zkochan added a commit that referenced this pull request Aug 4, 2023
zkochan added a commit that referenced this pull request Aug 4, 2023
: undefined
let downloaded = 0
const chunks: Buffer[] = []
res.body!.on('data', (chunk: Buffer) => {
Copy link
Copy Markdown
Contributor

@dmichon-msft dmichon-msft Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you aren't wrapping any callback-based APIs other than the data event here, you might find it convenient to take advantage of the fact that Node's Readable interface implements [Symbol.asyncIterator]:

// This will handle the 'data', 'error', and 'end' events.
for await (const chunk of res.body) {
  chunks.push(chunk);
  downloaded += chunk.length;
  onProgress?.(downloaded);
}

if (size !== null && size !== downloaded) {
  throw new BadTarballError(...);
}

// etc.

algorithm: 'sha512',
expected: 'sha1-HssnaJydJVE+rbyZFKc/VAi+enY=',
found: 'sha512-VuFL1iPaIxJK/k3gTxStIkc6+wSiDwlLdnCWNZyapsVLobu/0onvGOZolASZpfBFiDJYrOIGiDzgLIULTW61Vg== sha1-ACjKMFA7S6uRFXSDFfH4aT+4B4Y=',
found: 'sha1-ACjKMFA7S6uRFXSDFfH4aT+4B4Y=',
Copy link
Copy Markdown
Member Author

@zkochan zkochan Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why this test had to be changed but I guess it is OK. The error message is even better this way.

@zkochan zkochan marked this pull request as ready for review August 8, 2023 21:58
@zkochan zkochan merged commit 96e165c into main Aug 8, 2023
@zkochan zkochan deleted the fetch-perf branch August 8, 2023 22:41
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