Skip to content

script: Store DocumentLoader::blocking_loads in a HashSet instead of a Vec - #45974

Merged
jschwe merged 3 commits into
servo:mainfrom
Narfinger:document-loader-blocker
Jul 14, 2026
Merged

jschwe merged 3 commits into
servo:mainfrom
Narfinger:document-loader-blocker

Conversation

@Narfinger

@Narfinger Narfinger commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

This moves the BlockingLoads from Vector to a HashSet. This saves us roughly 0.3% on a sample run of the script thread execution as previously we had to search through the vector. As the LoadBlockers contain urls we use the std HashSet.

The only change in functionality is that we do not add the same blocker multiple times which could have happened in the previous case. This does not look like part of the design as blockers are removed at the same code point and do not get a handle.

Testing: This does not change observable behavior, so is covered by existing WPT tests.

@Narfinger
Narfinger requested a review from gterzian as a code owner June 25, 2026 09:07
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 25, 2026
@jschwe

jschwe commented Jun 25, 2026

Copy link
Copy Markdown
Member

How many entries could the vector have?

@Narfinger

Copy link
Copy Markdown
Contributor Author

Basically every subresource is a blocker such as images and scripts. Amazon for example has 120 at max (and it reduces obviously).

@Narfinger

Copy link
Copy Markdown
Contributor Author

@jschwe ping

@mrobinson mrobinson changed the title script: Move DocumentLoader::blocking_loads to hashset instead of vector script: Store DocumentLoader::blocking_loads in a HashSet instead of a Vec Jul 9, 2026
Comment thread components/script/document_loader.rs Outdated
#[derive(Clone, Debug, JSTraceable, MallocSizeOf, PartialEq)]
#[derive(Clone, Debug, Eq, Hash, JSTraceable, MallocSizeOf, PartialEq)]
pub(crate) enum LoadType {
Image(#[no_trace] ServoUrl),

@mrobinson mrobinson Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if two images are loading with the same URL? I think that could cause an issue here. You will need to store a count instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please check if this is how you meant it.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jul 9, 2026
Comment thread components/script/document_loader.rs Outdated
Comment thread components/script/document_loader.rs Outdated
@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 9, 2026
Narfinger added 2 commits July 9, 2026 15:38
Signed-off-by: Narfinger <[email protected]>
Signed-off-by: Narfinger <[email protected]>
@Narfinger
Narfinger force-pushed the document-loader-blocker branch from 377ce35 to a0e08f7 Compare July 9, 2026 13:39
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jul 9, 2026
Signed-off-by: Narfinger <[email protected]>
Comment thread components/script/document_loader.rs Outdated
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jul 10, 2026
@jschwe
jschwe added this pull request to the merge queue Jul 14, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 14, 2026
Merged via the queue into servo:main with commit 975b5d9 Jul 14, 2026
31 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 14, 2026
@Narfinger
Narfinger deleted the document-loader-blocker branch July 14, 2026 11:36
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.

4 participants