Skip to content

Conversation

jasnell
Copy link
Collaborator

@jasnell jasnell commented Jul 10, 2025

This is a refactor of the Headers implementation that improves construction benchmark performance.... The revised impl passes all WPT's and other tests and reduces per-header memory usage and memory accounting overhead while we're at it. Offering this as input to #4472

Copy link

codspeed-hq bot commented Jul 10, 2025

CodSpeed Performance Report

Merging #4494 will improve performances by ×2.2

Comparing jasnell/refactor-headers (1ea5b7c) with main (9b5895c)

Summary

⚡ 1 improvements
✅ 10 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
constructor[ApiHeaders] 156.1 ms 71 ms ×2.2

@jasnell jasnell force-pushed the jasnell/refactor-headers branch 7 times, most recently from 36784fb to 7802b6c Compare July 11, 2025 16:17
@jasnell jasnell marked this pull request as ready for review July 11, 2025 16:38
@jasnell jasnell requested review from a team as code owners July 11, 2025 16:38
@jasnell jasnell requested review from mikea and a team July 11, 2025 16:38
@anonrig
Copy link
Member

anonrig commented Jul 11, 2025

Can you move to the new file on a different PR? It's not possible to see what has changed when you move the whole implementation to a new file.

@jasnell
Copy link
Collaborator Author

jasnell commented Jul 11, 2025

The move to a different file is in a different commit already. That said, pretty much the entire impl is changed through these series of commits.

jasnell added 12 commits July 11, 2025 19:00
Remove the per-string accounting and instead account
per header entry.
When header names are set from our own code, and we know they are
valid, we can skip validation. If the header name is already known
to the Headers class we can also skip allocation.
When we know we are holding the isolate lock, we can avoid the
overhead of the deferred adjustment mechanism and just adjust
the isolate external memory directly.
@jasnell jasnell force-pushed the jasnell/refactor-headers branch from 0ab6472 to 5f01fd8 Compare July 12, 2025 02:00
@jasnell jasnell force-pushed the jasnell/refactor-headers branch from 5f01fd8 to 1ea5b7c Compare July 12, 2025 02:15
@jasnell jasnell merged commit 697b978 into main Jul 12, 2025
32 of 33 checks passed
@jasnell jasnell deleted the jasnell/refactor-headers branch July 12, 2025 13:55
jasnell added a commit that referenced this pull request Aug 19, 2025
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