Skip to content

Custom limits for dynamic workers#6399

Merged
git-bruh merged 1 commit into
mainfrom
pkhanna/custom-dynamic-worker-limits
Apr 13, 2026
Merged

Custom limits for dynamic workers#6399
git-bruh merged 1 commit into
mainfrom
pkhanna/custom-dynamic-worker-limits

Conversation

@git-bruh

@git-bruh git-bruh commented Mar 24, 2026

Copy link
Copy Markdown
Member

No description provided.

@github-actions

github-actions Bot commented Mar 24, 2026 •
edited
Loading

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@git-bruh

git-bruh commented Mar 24, 2026

Copy link
Copy Markdown
Member Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Mar 24, 2026
@github-actions
@git-bruh has signed the CLA in #6399
58b84fa
ask-bonk[bot]
ask-bonk Bot reviewed Mar 24, 2026
View reviewed changes

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR adds a ResourceLimits struct (with cpuMs and subRequests) and plumbs it through the dynamic worker loading and entrypoint/actor-class APIs. I found three issues, two of which are high severity.

  1. [HIGH] Build break: WorkerStubImpl in server.c++:4107-4114 does not accept the new limits parameter, so the override keyword will fail to compile against the updated pure virtual interface in WorkerStubChannel.
  2. [HIGH] Bug in DynamicWorkerSource::clone(): The new .limits field is not propagated by the clone() method (io-channels.h:361-372), so cloned sources silently lose their limits. This code path is exercised by WorkerLoader::load(), which calls source.clone() inside a closure that may be invoked multiple times.
  3. [LOW] No tests: There are no tests covering the new limits plumbing — neither for WorkerCode.limits flowing into DynamicWorkerSource, nor for EntrypointOptions.limits flowing into getEntrypoint()/getActorClass().

This review was generated by an AI assistant and may contain inaccuracies.

Comment thread src/workerd/io/io-channels.h
Comment thread src/workerd/io/io-channels.h
@github-actions

github-actions Bot commented Mar 24, 2026 •
edited
Loading

Copy link
Copy Markdown

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@ask-bonk

ask-bonk Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Review posted successfully on PR #6399. Here's a summary of the findings:

  1. [HIGH] Build break: WorkerStubImpl in server.c++ overrides getEntrypoint and getActorClass with the old two-parameter signatures, but the base class WorkerStubChannel now requires a third kj::Maybe<ResourceLimits> limits parameter. The override keyword will cause a compilation failure.

  2. [HIGH] DynamicWorkerSource::clone() drops limits: The clone() method does not include the new .limits field in its designated-initializer list, so cloned sources silently lose their resource limits. This affects the WorkerLoader::load() path.

  3. [LOW] No tests for the new limits plumbing.

github run

@git-bruh git-bruh force-pushed the pkhanna/custom-dynamic-worker-limits branch from 6dcfdce to 7588383 Compare March 24, 2026 16:39
@codspeed-hq

codspeed-hq Bot commented Mar 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 70 untouched benchmarks
⏩ 129 skipped benchmarks1

Comparing pkhanna/custom-dynamic-worker-limits (f766a8e) with main (cd34764)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

kentonv
kentonv approved these changes Mar 24, 2026
View reviewed changes
Comment thread src/workerd/io/io-channels.h
@git-bruh git-bruh force-pushed the pkhanna/custom-dynamic-worker-limits branch 2 times, most recently from 0f4f880 to 77f9e20 Compare March 25, 2026 07:01
@git-bruh git-bruh marked this pull request as ready for review March 25, 2026 07:02
@git-bruh git-bruh requested review from a team as code owners March 25, 2026 07:02
@git-bruh git-bruh requested a review from edmundhung March 25, 2026 07:02
@git-bruh git-bruh force-pushed the pkhanna/custom-dynamic-worker-limits branch 2 times, most recently from d1dbd0f to 87e26eb Compare March 31, 2026 09:31
@git-bruh git-bruh force-pushed the pkhanna/custom-dynamic-worker-limits branch from 87e26eb to 2a95aea Compare April 6, 2026 06:23
@git-bruh git-bruh force-pushed the pkhanna/custom-dynamic-worker-limits branch 2 times, most recently from 8dcc3c0 to 2e07e96 Compare April 11, 2026 05:50
@git-bruh git-bruh force-pushed the pkhanna/custom-dynamic-worker-limits branch from 2e07e96 to f766a8e Compare April 13, 2026 08:12
@git-bruh git-bruh requested a review from jamesopstad April 13, 2026 10:33
@git-bruh git-bruh enabled auto-merge (rebase) April 13, 2026 12:21
@git-bruh git-bruh merged commit 8765a37 into main Apr 13, 2026
24 of 25 checks passed
@git-bruh git-bruh deleted the pkhanna/custom-dynamic-worker-limits branch April 13, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ask-bonk ask-bonk[bot] ask-bonk[bot] left review comments

@jasnell jasnell jasnell approved these changes

@kentonv kentonv kentonv approved these changes

@jamesopstad jamesopstad jamesopstad approved these changes

@edmundhung edmundhung Awaiting requested review from edmundhung edmundhung is a code owner automatically assigned from cloudflare/wrangler

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@git-bruh @jasnell @kentonv @jamesopstad