Conversation
Opening any remote workload outside the harness threw a TypeError on the first stray message instead of logging it. Note that I didn't rebuild the existing remote workloads. I checked the others and it looks like they show a bit diff in the bundle, so I chose to leave them for now. The next rebuild will take this up.
`StepRunner` only awaited the step body when its "type" argument was "async". `AsyncBenchmarkStep` created an `AsyncStepRunner` without passing that argument, so the step promise was never awaited and an async remote step measured ~0ms with no error. This commit makes the runner class decide instead: an `isAsync` getter is false on `StepRunner` and true on `AsyncStepRunner`, and the type argument is removed. Sync steps stay a plain call, since awaiting them would add a microtask hop to the measured time. The bug was never hit because nothing in the tree or in the open workload PRs uses `AsyncBenchmarkStep`. But this is a requirement for the pdf.js workload that I'm working on.
A document viewer on the pdf.js core API. It covers work like: canvas 2D drawing, rasterising glyphs from embedded fonts and JPEG decoding. The suite is `remote` with a single async step that renders `6 * complexity` pages and their thumbnails. Decisions worth knowing about: - It uses the core API rather than pdf.js' `pdf_viewer.mjs`, whose render queue and scroll handling schedule work with setTimeout that would land inside the measured step. - pdf.js continues each render chunk on requestAnimationFrame, which ties the step to the display's refresh rate. The step swaps rAF for a timer while it runs, and restores it after. - Nothing is rendered before the step, since pdf.js caches operator lists per page and an early render would leave the step a warm cache. - The fixture PDF is generated at build time with PDFKit, so its content is reviewable as code and byte-identical between builds.
✅ Deploy Preview for webkit-speedometer-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a new experimental workload called
PDFViewer-PDFjswhich displays a PDF using PDF.js.This PR builds on top of two PRs that fix some benchmark related issues: #588 and #589. Currently it includes their commits too, and I'll rebase it once they land.
Some information about the workload:
A document viewer on the pdf.js core API. It covers work like: canvas 2D drawing, rasterising glyphs from embedded fonts and JPEG decoding. The suite is
remotewith a single async step that renders6 * complexitypages and their thumbnails.Decisions worth knowing about:
pdf_viewer.mjs, whose render queue and scroll handling schedule work with setTimeout that would land inside the measured step.