Avoid more allocations when compiling html5ever #37373
Conversation
| @@ -161,7 +162,7 @@ pub fn count_names(ms: &[TokenTree]) -> usize { | |||
| }) | |||
| } | |||
|
|
|||
| pub fn initial_matcher_pos(ms: Rc<Vec<TokenTree>>, sep: Option<Token>, lo: BytePos) | |||
| pub fn initial_matcher_pos(ms: Vec<TokenTree>, sep: Option<Token>, lo: BytePos) | |||
There was a problem hiding this comment.
I believe this means that we are now copying/moving the Vec; perhaps we only need &[TokenTree] here?
There was a problem hiding this comment.
The Vec gets put into the MatcherPos, so if it becomes a reference then MatcherPos will need a lifetime paramter and things get more complicated. Copying a Vec shouldn't be that expensive because only three words get copied, right? I did a Cachegrind run and this commit caused the number of instructions executed to drop.
There was a problem hiding this comment.
Cloning a Vec copies all the allocated elements. But it can probably use memcpy, so that should be fast.
There was a problem hiding this comment.
I was under the impression that passing a Vec as an argument to a function doesn't clone it. Have I got that wrong?
There was a problem hiding this comment.
oh right... forget I said anything, I thought the Rc was there to prevent the cloning.
|
Are those changes speeding up the compilation of other programs too? Optimizations should be added only after there's a more general proof of their quality. |
|
It looks like SmallVec should impl Deref/DerefMut so that the .as_slice()/mut calls become redundant, and it's more of a drop in replacement for Vec. |
| .cloned() | ||
| .collect()), | ||
| let mut cur_eis = SmallVector::zero(); | ||
| cur_eis.push(initial_matcher_pos(ms.iter().cloned().collect(), |
There was a problem hiding this comment.
this should be a simple ms.to_owned()
There was a problem hiding this comment.
And could this use SmallVector::one instead of zero and push?
|
r=me with the nits in macro_parser.rs addressed |
nnethercote
force-pushed
the
html5ever-more-more
branch
from
74ecc3f to
d37650a
Compare
This avoids 800,000 heap allocations when compiling html5ever.
This avoids 800,000 heap allocations when compiling html5ever. It requires tweaking `SmallVector` a little.
This avoids 800,000 allocations when compiling html5ever.
nnethercote
force-pushed
the
html5ever-more-more
branch
from
d37650a to
c440a7a
Compare
|
Updated to address comments, including adding |
|
@bors: r+ |
|
📌 Commit c440a7a has been approved by |
Avoid more allocations when compiling html5ever These three commits reduce the number of allocations performed when compiling html5ever from 13.2M to 10.8M, which speeds up compilation by about 2%. r? @nrc
Avoid more allocations when compiling html5ever These three commits reduce the number of allocations performed when compiling html5ever from 13.2M to 10.8M, which speeds up compilation by about 2%. r? @nrc
brson
added
the
relnotes
These three commits reduce the number of allocations performed when compiling html5ever from 13.2M to 10.8M, which speeds up compilation by about 2%.
r? @nrc