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
Use a 1-element smallvec for selector lists. #18395
Conversation
r? @emilio |
components/selectors/parser.rs
Outdated
} | ||
|
||
/// Creates a SelectorList from a Vec of selectors. Used in tests. | ||
pub fn from_vec(v: Vec<Selector<Impl>>) -> Self { | ||
SelectorList(v) | ||
SelectorList(v.into_iter().collect()) |
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.
.into_vec()
reuses an existing allocation if there is one.
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 misread what this does. SmallVec::from_vec
, then.
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.
Yeah, it's not a big deal, this is just for testing anyway.
r=me with this fixed: error: expected one of `,` or `>`, found `;`
--> /home/travis/build/servo/servo/components/selectors/parser.rs:179:74
|
179 | pub struct SelectorList<Impl: SelectorImpl>(pub SmallVec<[Selector<Impl>]; 1>);
| ^ expected one of `,` or `>` here
error: expected item, found `1`
--> /home/travis/build/servo/servo/components/selectors/parser.rs:179:76
|
179 | pub struct SelectorList<Impl: SelectorImpl>(pub SmallVec<[Selector<Impl>]; 1>);
| ^ |
2582ce8
to
3ae7781
Compare
@bors-servo: r=SimonSapin |
📌 Commit 3ae7781 has been approved by |
Use a 1-element smallvec for selector lists. Profiling shows this reducing parsing time by a few milliseconds on the tp6 facebook case. The gtest benchmark with the same concatenated stylesheets also shows a significant improvement. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] There are tests for these changes <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18395) <!-- Reviewable:end -->
💔 Test failed - linux-rel-wpt |
@bors-servo: retry |
Use a 1-element smallvec for selector lists. Profiling shows this reducing parsing time by a few milliseconds on the tp6 facebook case. The gtest benchmark with the same concatenated stylesheets also shows a significant improvement. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] There are tests for these changes <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18395) <!-- Reviewable:end -->
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev |
Profiling shows this reducing parsing time by a few milliseconds on the tp6 facebook case. The gtest benchmark with the same concatenated stylesheets also shows a significant improvement.
./mach build -d
does not report any errors./mach test-tidy
does not report any errorsThis change is