Skip to content

Comments

Shrink JsWorkerRequest & use the right HashMap/Set#4150

Merged
Centril merged 1 commit intomasterfrom
centril/shrink-jsreq
Jan 29, 2026
Merged

Shrink JsWorkerRequest & use the right HashMap/Set#4150
Centril merged 1 commit intomasterfrom
centril/shrink-jsreq

Conversation

@Centril
Copy link
Contributor

@Centril Centril commented Jan 28, 2026

Description of Changes

This PR shrinks JsWorkerRequest so that it is (almost) as small as the call reducer request.
To do that, a bunch of trivial changes had to be done to auth code, that mostly revolves around String -> Box<str>.
This should help the auth code, but that is incidental.
The main goal was to improve throughput through the request tx/rx channel for V8, which is taking quite a bit of time in flamegraphs.

I also noticed while making this change that the wrong hash map was being used in a bunch of places, so I fixed all of those.

A follow up PR will shrink the reply side to fit within a cache line.
Yet another follow up PR will change the channel to replace flume with fibre::spsc.

API and ABI breaking changes

None

Expected complexity level and risk

2, fairly trivial changes.

Testing

Covered by existing tests.

Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

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

my code-owned changes (CLI) look good to me, although I don't know that this distinction really matters in there since it's very not-performance-sensitive.

I don't think I'm the right reviewer for most of the rest of the codebase.

@Centril Centril force-pushed the centril/shrink-jsreq branch from 8872018 to b11287e Compare January 29, 2026 08:14
@Centril Centril enabled auto-merge January 29, 2026 08:14
@Centril Centril added this pull request to the merge queue Jan 29, 2026
Merged via the queue into master with commit 3d3c99f Jan 29, 2026
27 of 28 checks passed
@Centril Centril deleted the centril/shrink-jsreq branch January 29, 2026 09:38
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.

3 participants