Reduce per-request CPU/memory in workspace-package check and tarball fetch#49
Merged
Conversation
…ball downloads into one buffer isWorkspacePackage re-split/trimmed WORKSPACE_PACKAGES on every call even though it's a static Worker var; memoize it (and share the split/trim/filter parse with parseConfiguredPreviewRefs via a new splitCsv util). fetchUpstreamTarball collected chunks into an array then copied them into a final buffer. When Content-Length is known (always, from pkg.pr.new), stream straight into one pre-sized buffer instead, halving peak memory for that path.
… cap
The xhigh code review flagged that the pre-sized buffer path enforced the
upstream's Content-Length header as an exact byte ceiling, so a legitimate
small tarball could be rejected with a spurious 413 if the header slightly
under-reported the real body (a re-chunking proxy, an off-by-a-few-bytes
value), or if the header was blank (Number('') coerces to 0, not NaN).
Unify the pre-sized and fallback paths into one growable buffer: pre-size
from Content-Length as a hint (validated as a strict non-negative integer),
but only ever reject on the actual maxBytes limit, growing the buffer when
the declared size undershoots. Rebuilt the action bundle (publish-preview
imports isWorkspacePackage/parseConfiguredPreviewRefs, both changed in the
prior commit).
Codex review flagged that unifying both paths into one growable buffer made the truly-unknown-length case (no Content-Length at all) start from a 64KiB guess and double up to the maxBytes cap before slicing back down, so a body just over half of maxBytes could transiently allocate close to 2x maxBytes, worse than the original chunk-array approach's ~2x actual-size peak. Keep the pre-sized/grow-on-undershoot buffer only for when a Content-Length hint exists (where a wrong guess is expected to be a small correction, not an order-of-magnitude gap); the no-hint case goes back to collecting chunks and copying once into an exactly-sized buffer at the end, so peak memory tracks the actual body size instead of a doubling sequence that can overshoot it.
3 tasks
fengmk2
added a commit
that referenced
this pull request
Jul 1, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isWorkspacePackagere-split/trimmedWORKSPACE_PACKAGESon every call even though it's a static Worker var for the isolate's lifetime; memoize it, sharing the split/trim/filter parse withparseConfiguredPreviewRefsvia a newsplitCsvutil.fetchUpstreamTarballcollected chunks into an array then copied them into a second, final buffer. Upstream responses always declareContent-Length, so it now streams straight into one pre-sized buffer in that case, halving peak memory for the common path (falls back to the old approach when it's absent).Test plan
pnpm typecheckpnpm test(82/82 passing, 5 new tests covering the pre-sized/fallback paths and all size-guard cases)