CodSpeed Performance Report

Merging #5396 will improve performances by 44.01%

_{Comparing jasnell/http-headers-take-2 (00269d5) with main (1711af3)}

Summary

⚡ 2 improvements
✅ 55 untouched
⏩ 30 skipped¹

Benchmarks breakdown

Internal tests are failing because the refactor changes the ordering of the headers in places... will need to see if we can preserve the order but first want to see if this has a positive impact on performance overall... waiting for benchmark results to be updated.

Marking as ready for review but there are still some internal test updates I'll need to do on the internal repo.

Whenever this is done I want as many reviewers as we can get on this, incl Harris, Kenton, Mike, Yagiz, etc.

Note for reviewers: while it likely is possible to get more performance gain with additional tweaks on this, the emphasis for review should be on correctness and ensuring that the revised implementation does not introduce new bugs, etc. We can tweak additional performance knobs separately in follow up PRs.

The only implementation I saw in the diff for checkGuard is:

void checkGuard() {
    JSG_REQUIRE(guard == Guard::NONE, TypeError, "Can't modify immutable headers.");
  }

which doesn't include forbidden headers, but please let me know if I'm missing anything. My question was specifically about forbidden headers and I completely understand if they are omitted, was just trying to point out gaps.

ah, right, yeah we don't (and have never) implemented the forbidden headers bit.

The force-push https://github.com/cloudflare/workerd/compare/d55e9e51ffe9a606bc70ddd2353c82fd8979f08c..76e21b53212977137242f65a82b664444181a611 seems to contain a combination of changes specific to this PR, and also a rebase on master, which breaks incremental reviews.