-
Notifications
You must be signed in to change notification settings - Fork 777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support profiling the startup phase of a Worker #8026
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 989f763 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
50f8524
to
682fc9b
Compare
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-wrangler-8026 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/8026/npm-package-wrangler-8026 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-wrangler-8026 dev path/to/script.js Additional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-workers-bindings-extension-8026 -O ./cloudflare-workers-bindings-extension.0.0.0-vd6e483add.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vd6e483add.vsix create-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-create-cloudflare-8026 --no-auto-update @cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-kv-asset-handler-8026 miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-miniflare-8026 @cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-pages-shared-8026 @cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-unenv-preset-8026 @cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-vite-plugin-8026 @cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-vitest-pool-workers-8026 @cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-workers-editor-shared-8026 @cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-workers-shared-8026 @cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13265276676/npm-package-cloudflare-workflows-shared-8026 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
71e85b3
to
07d1f70
Compare
d6c31a2
to
650bd73
Compare
650bd73
to
6d62eeb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pages build already has an outfile, is this supposed to reuse that...? Can pages deploy fail due to startup time, and should this generate a cpuprofile if that's the case?
|
||
const mimeTypeModuleType = flipObject(moduleTypeMimeType); | ||
|
||
export const checkNamespace = createNamespace({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not blocking, especially since its hidden, but it feels odd this has its own command. Do we anticipate other 'check' commands?
...maybe we should make wrangler deploy --dry-run
a check command?? (come to think of it, maybe we should resurrect wrangler build
for wrangler deploy --dry-run
🤔)
(these are only semi-serious suggestions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree–we should definitely resurrect wrangler build
!
In terms of the command structure here, I went with check startup
since it seemed feasible that we might want to have other check *
commands in future (check types
? check config
?), but since this is an alpha command we can always change that later.
Co-authored-by: emily-shen <[email protected]>
It's meant to be doing that, yeah—does that seem buggy to you? |
Ah no that’s fine, the lack of changes in the pages deploy path just threw me for a moment but it makes sense now. Will the cpuprofile be generated automatically if a pages deploy fails? |
This PR adds two features (separate changesets):
--outfile
argument forwrangler deploy
, which outputs the exact Worker bundle that Wrangler will upload to the Cloudflare API. This has more information than just--outdir
because it bakes in bindings & module types.wrangler check startup
, which profiles the startup time of a Worker and generates a.cpuprofile
file that can be opened in Chrome DevTools/directly in VSCode for further investigation. Additionally, this is automatically generated when Wrangler encounters a startup time error from the API.wrangler check startup
docs cloudflare-docs#19877