| CARVIEW |
Select Language
HTTP/2 200
date: Mon, 29 Dec 2025 02:32:10 GMT
content-type: text/html; charset=utf-8
cache-control: no-cache
content-security-policy: default-src 'none'; base-uri 'self'; child-src github.githubassets.com github.com/assets-cdn/worker/ github.com/assets/ gist.github.com/assets-cdn/worker/; connect-src 'self' uploads.github.com www.githubstatus.com collector.github.com raw.githubusercontent.com api.github.com github-cloud.s3.amazonaws.com github-production-repository-file-5c1aeb.s3.amazonaws.com github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com *.rel.tunnels.api.visualstudio.com wss://*.rel.tunnels.api.visualstudio.com github.githubassets.com objects-origin.githubusercontent.com copilot-proxy.githubusercontent.com proxy.individual.githubcopilot.com proxy.business.githubcopilot.com proxy.enterprise.githubcopilot.com *.actions.githubusercontent.com wss://*.actions.githubusercontent.com productionresultssa0.blob.core.windows.net/ productionresultssa1.blob.core.windows.net/ productionresultssa2.blob.core.windows.net/ productionresultssa3.blob.core.windows.net/ productionresultssa4.blob.core.windows.net/ productionresultssa5.blob.core.windows.net/ productionresultssa6.blob.core.windows.net/ productionresultssa7.blob.core.windows.net/ productionresultssa8.blob.core.windows.net/ productionresultssa9.blob.core.windows.net/ productionresultssa10.blob.core.windows.net/ productionresultssa11.blob.core.windows.net/ productionresultssa12.blob.core.windows.net/ productionresultssa13.blob.core.windows.net/ productionresultssa14.blob.core.windows.net/ productionresultssa15.blob.core.windows.net/ productionresultssa16.blob.core.windows.net/ productionresultssa17.blob.core.windows.net/ productionresultssa18.blob.core.windows.net/ productionresultssa19.blob.core.windows.net/ github-production-repository-image-32fea6.s3.amazonaws.com github-production-release-asset-2e65be.s3.amazonaws.com insights.github.com wss://alive.github.com wss://alive-staging.github.com api.githubcopilot.com api.individual.githubcopilot.com api.business.githubcopilot.com api.enterprise.githubcopilot.com; font-src github.githubassets.com; form-action 'self' github.com gist.github.com copilot-workspace.githubnext.com objects-origin.githubusercontent.com; frame-ancestors 'none'; frame-src viewscreen.githubusercontent.com notebooks.githubusercontent.com; img-src 'self' data: blob: github.githubassets.com media.githubusercontent.com camo.githubusercontent.com identicons.github.com avatars.githubusercontent.com private-avatars.githubusercontent.com github-cloud.s3.amazonaws.com objects.githubusercontent.com release-assets.githubusercontent.com secured-user-images.githubusercontent.com/ user-images.githubusercontent.com/ private-user-images.githubusercontent.com opengraph.githubassets.com marketplace-screenshots.githubusercontent.com/ copilotprodattachments.blob.core.windows.net/github-production-copilot-attachments/ github-production-user-asset-6210df.s3.amazonaws.com customer-stories-feed.github.com spotlights-feed.github.com objects-origin.githubusercontent.com *.githubusercontent.com; manifest-src 'self'; media-src github.com user-images.githubusercontent.com/ secured-user-images.githubusercontent.com/ private-user-images.githubusercontent.com github-production-user-asset-6210df.s3.amazonaws.com gist.github.com github.githubassets.com; script-src github.githubassets.com; style-src 'unsafe-inline' github.githubassets.com; upgrade-insecure-requests; worker-src github.githubassets.com github.com/assets-cdn/worker/ github.com/assets/ gist.github.com/assets-cdn/worker/
referrer-policy: no-referrer-when-downgrade
server-timing: pull_request_layout-fragment;desc="pull_request_layout fragment";dur=557.437525,conversation_content-fragment;desc="conversation_content fragment";dur=2289.301485,conversation_sidebar-fragment;desc="conversation_sidebar fragment";dur=355.580483,nginx;desc="NGINX";dur=0.850167,glb;desc="GLB";dur=96.83126
strict-transport-security: max-age=31536000; includeSubdomains; preload
vary: X-PJAX, X-PJAX-Container, Turbo-Visit, Turbo-Frame, X-Requested-With,Accept-Encoding, Accept, X-Requested-With
x-content-type-options: nosniff
x-frame-options: deny
x-voltron-version: aab62e3
x-xss-protection: 0
server: github.com
content-encoding: gzip
accept-ranges: bytes
set-cookie: _gh_sess=vQ9jXNjny41DcAgEMbiN%2F1xkdcnebAavIbvyHoen5Ara5KyWsdwXyfMLveBFH3zvi6QP%2Boj04%2FV5wlQQ0GmN%2F1ipfORDKS4ce4irTxBdOKuGQ83Fxx7STJOPvQhWP85rt1yfxTnpXvFUrDPYHaaZq61ItfSJ6IZb1jvnXCxDZPrRk4gvODiPwrxlEuijbje6lq6pcGTbzgiUnRdabmykzZUXpBNOT0HYHHjcCHXTrQqd74dAF4KcbrnWSRzgTLjWIbOrTwPKz%2FfFKf1OA5DY9A%3D%3D--SsOt27TlM3vafBaF--nUFoG54uSlMOqudvUNy4DA%3D%3D; Path=/; HttpOnly; Secure; SameSite=Lax
set-cookie: _octo=GH1.1.361734079.1766975527; Path=/; Domain=github.com; Expires=Tue, 29 Dec 2026 02:32:07 GMT; Secure; SameSite=Lax
set-cookie: logged_in=no; Path=/; Domain=github.com; Expires=Tue, 29 Dec 2026 02:32:07 GMT; HttpOnly; Secure; SameSite=Lax
x-github-request-id: D80E:2B1209:566166D:68A02EC:6951E827
script: Add support for parsing CSS in parallel by mrobinson · Pull Request #40639 · servo/servo · GitHub
servo-highfive
added
the
S-awaiting-review
There is new code that needs to be reviewed.
label
Nov 14, 2025
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
instead to consolidate mime type handling
Copy link
Copy link
Copy link
Copy link
Copy link
servo-highfive
removed
the
S-awaiting-review
There is new code that needs to be reviewed.
label
Nov 14, 2025
mrobinson
force-pushed
the
asynchronous-css-parsing
branch
from
November 14, 2025 15:35
servo-highfive
added
the
S-awaiting-review
There is new code that needs to be reviewed.
label
Nov 14, 2025
servo-highfive
added
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Nov 14, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Nov 14, 2025
servo-highfive
added
S-tests-failed
The changes caused existing tests to fail.
and removed
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
S-tests-failed
The changes caused existing tests to fail.
labels
Nov 14, 2025
Copy link
Copy link
Copy link
mrobinson
force-pushed
the
asynchronous-css-parsing
branch
from
November 14, 2025 21:21
servo-highfive
added
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Nov 14, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Nov 14, 2025
servo-highfive
added
S-tests-failed
The changes caused existing tests to fail.
and removed
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
labels
Nov 14, 2025
mrobinson
force-pushed
the
asynchronous-css-parsing
branch
from
November 15, 2025 08:04
servo-highfive
removed
the
S-tests-failed
The changes caused existing tests to fail.
label
Nov 15, 2025
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
servo-highfive
added
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Nov 15, 2025
servo-highfive
removed
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Nov 15, 2025
Skip to content
Navigation Menu
{{ message }}
-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
script: Add support for parsing CSS in parallel #40639
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
Merged
Merged
Conversation
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
servo-highfive
added
the
S-awaiting-review
|
🔨 Triggering try run (#19358341926) for Linux (WPT) |
Member
|
Did you run any tests to check the performance impact? |
|
Test results for linux-wpt from try job (#19358341926): Flaky unexpected result (49)
Stable unexpected results that are known to be intermittent (29)
|
|
✨ Try run (#19358341926) succeeded. |
Member
Author
|
I have not run performance tests and I suspect it depends on the hardware Servo is running on. The idea here is to add this support and then disable or enable it based on later performance testing. I'm very curious to hear about how this affects performance on embedded systems, for instance. |
TimvdLippe
approved these changes
Nov 14, 2025
Contributor
TimvdLippe
left a comment
TimvdLippe
left a comment
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.
Overall this LGTM! I mainly have some nits and clarifying questions about code structure. Glad to see this land 😄
| // From <https://html.spec.whatwg.org/multipage/#link-type-stylesheet>: | ||
| // > A link element of this type is implicitly potentially render-blocking if the element | ||
| // > was created by its node document's parser. | ||
| if matches!(self.source, StylesheetContextSource::LinkElement) && owner.parser_inserted() { |
Contributor
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.
Nit: move this if into the other if so we only check owner.parser_inserted() once
Member
Author
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.
Oh, I meant to do this earlier, but I didn't get around to it. I've added an early return here.
| .expect("Stylesheet not loaded by <style> or <link> element!"); | ||
|
|
||
| if self.is_for_a_previous_generation(&element) { | ||
| self.decrement_load_and_render_blockers(owner, &document); |
Contributor
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.
Should we also return early here, since we shouldn't process it any longer?
Member
Author
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.
Yes! Thank you for spotting this. I meant to do an early return here. This would have caused all kinds of chaos in the future as-is.
| document.invalidate_stylesheets(); | ||
| } | ||
|
|
||
| self.decrement_load_and_render_blockers(owner, &document); |
Contributor
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.
If we don't early return above, then why do we decrement twice here?
Member
Author
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.
👍
Comment on lines
230
to
236
| let event = match any_failed { | ||
| true => atom!("error"), | ||
| false => atom!("load"), | ||
| }; |
Contributor
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.
Ultra-nit: why not use if instead of match on a boolean?
Suggested change
| let event = match any_failed { | |
| true => atom!("error"), | |
| false => atom!("load"), | |
| }; | |
| let event = if any_failed { atom!("error") } else { atom!("load") }; |
Member
Author
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 went with a match because it was one less line, but your approach is even better.
Member
Author
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.
Hrm. It seems that rustfmt doesn't like the single-line version, so maybe I'll stick with the match since it is one less line than this:
let event = match any_failed {
true => atom!("error"),
false => atom!("load"),
};
Comment on lines
293
to
294
| let mime: Mime = content_type.into_inner().into(); | ||
| mime.type_() == mime::TEXT && mime.subtype() == mime::CSS |
Contributor
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.
Nit: consider using
servo/components/shared/net/mime_classifier.rs
Lines 323 to 325 in 178e6b9
| fn is_css(mt: &Mime) -> bool { | |
| mt.essence_str() == "text/css" | |
| } |
Member
Author
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.
Okay. I've made this public and reused it.
| // > external resource is not a supported style sheet type, the user agent must | ||
| // > instead assume it to be text/css. | ||
| if document.quirks_mode() == QuirksMode::Quirks && | ||
| document.url().origin() == self.url.origin() |
Contributor
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.
Why do we need to do this twice? Above we are checking final_url and here we are checking url. Can we consolidate these or remove one of the branches maybe?
Member
Author
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.
Looks like this is due to two PRs doing the same thing landing around the same time, but one originating from around 4 years ago. I landed the old one (after the new one) when I was trying to adopt old PRs. It seems I made a mistake in not removing the duplicate check. Since this existed before my change, I'll preserve it, but try to open a followup to fix it.
Here are the two PRs:
- Fix: Add support for stylesheet MIME type quirk in quirks mode #36338 (newer, landed first)
- Use spec compliant content-type extraction in more places and enable a
<stylesheet>quirk #28321 (older, I landed second and didn't notice the duplicate check)
Comment on lines
327
to
332
| let loader = match pref!(dom_parallel_css_parsing_enabled) { | ||
| true => { | ||
| ElementStylesheetLoader::Asynchronous(AsynchronousStylesheetLoader::new(&element)) | ||
| }, | ||
| false => ElementStylesheetLoader::Synchronous { element: &element }, | ||
| }; |
Contributor
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.
Same ultra-nit about matching booleans:
Suggested change
| let loader = match pref!(dom_parallel_css_parsing_enabled) { | |
| true => { | |
| ElementStylesheetLoader::Asynchronous(AsynchronousStylesheetLoader::new(&element)) | |
| }, | |
| false => ElementStylesheetLoader::Synchronous { element: &element }, | |
| }; | |
| let loader = if pref!(dom_parallel_css_parsing_enabled) { | |
| ElementStylesheetLoader::Asynchronous(AsynchronousStylesheetLoader::new(&element)) | |
| } else { | |
| ElementStylesheetLoader::Synchronous { element: &element } | |
| }; |
Member
Author
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.
This makes a lot of sense. I'll make the change.
servo-highfive
removed
the
S-awaiting-review
This change is a rework of servo#22478, originally authored by @vimpunk. It adds parsing of CSS in parallel with the main script thread. The big idea here is that when the transfer of stylesheet bytes is finished, the actual parsing is pushed to a worker thread from the Stylo thread pool. This also applies for subsequent loads triggered by `@import` statements. The design is quite similar to the previous PR with a few significant changes: - Error handling works properly. The `CSSErrorReporter` is a crossbeam `Sender` and a `PipelineId` so it can be trivially cloned and sent to the worker thread. - Generation checking is done both before and after parsing, in order to both remove the race condition and avoid extra work when the generations do not match. - The design is reworked a bit to avoid code duplication, dropping added lines from 345 to 160. - Now that `process_response_eof` gives up ownership to the `FetchResponseListener`, this change avoids all extra copies. Co-authored-by: mandreyel <mandreyel@protonmail.com> Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson
force-pushed
the
asynchronous-css-parsing
branch
from
189b70b to
30c3b2d
Compare
servo-highfive
added
the
S-awaiting-review
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
|
🔨 Triggering try run (#19372634583) for Linux (WPT) |
|
|
|
🔨 Triggering try run (#19377936064) for Linux (WPT) |
mrobinson
force-pushed
the
asynchronous-css-parsing
branch
from
3af54ed to
1311cce
Compare
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
- Cancel load blockers when exited early due to generation change - Make generation change more general by having the method check whether the element still contributes to the styling processing model in particular whether it is still connected to the DOM. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson
force-pushed
the
asynchronous-css-parsing
branch
from
1311cce to
d873cc3
Compare
servo-highfive
removed
the
S-tests-failed
|
🔨 Triggering try run (#19386855605) for Linux (WPT) |
Member
Author
|
Hrm. So it seems that the old code was relying on "ignored" loads to finish in order to send events, depending on the order that things arrived in. We hit this path more with the extra step of asynchronicity. I think that this should be a bit more closely tied to the generation concept that the |
|
Test results for linux-wpt from try job (#19386855605): Flaky unexpected result (43)
Stable unexpected results that are known to be intermittent (31)
|
|
✨ Try run (#19386855605) succeeded. |
|
🔨 Triggering try run (#19387255095) for Linux (WPT) |
|
Test results for linux-wpt from try job (#19387255095): Flaky unexpected result (39)
Stable unexpected results that are known to be intermittent (29)
|
|
✨ Try run (#19387255095) succeeded. |
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Loirooriol
added a commit
to Loirooriol/servo
that referenced
this pull request
Nov 24, 2025
After servo#40639, this variant holds a single value, and I don't think think using named fields is providing much. Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Loirooriol
added a commit
to Loirooriol/servo
that referenced
this pull request
Nov 24, 2025
After servo#40639, this variant holds a single value, and I don't think think using named fields was providing much extra clarity. Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Loirooriol
added a commit
to Loirooriol/servo
that referenced
this pull request
Nov 24, 2025
After servo#40639, this variant holds a single value, and I don't think think using named fields was providing much extra clarity. Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
You can’t perform that action at this time.
This change is a rework of #22478, originally authored by @vimpunk.
It adds parsing of CSS in parallel with the main script thread. The
big idea here is that when the transfer of stylesheet bytes is
finished, the actual parsing is pushed to a worker thread from the Stylo
thread pool. This also applies for subsequent loads triggered by
@importstatements.The design is quite similar to the previous PR with a few significant
changes:
CSSErrorReporteris a crossbeamSenderand aPipelineIdso it can be trivially cloned and sent tothe worker thread.
to both remove the race condition and avoid extra work when the
generations do not match.
lines from 345 to 160.
process_response_eofgives up ownership to theFetchResponseListener, this change avoids all extra copies.Testing: This shouldn't change observable behavior, so is covered
by existing tests.
Fixes: #20721
Closes: #22478