Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

legalizer/split.rs: simplify_branch_arguments: use SmallVec instead o… #999

Merged
merged 1 commit into from Sep 9, 2019
Merged

legalizer/split.rs: simplify_branch_arguments: use SmallVec instead o… #999

merged 1 commit into from Sep 9, 2019

Conversation

julian-seward1
Copy link
Collaborator

…f Vec

This function is responsible for 8.5% of all heap allocation (calls) in CL.
This change avoids almost all of them by using a SmallVec::<[Value; 32]>
instead. Dynamic instruction count falls by 0.25%. The fixed size of 32 was
arrived at after profiling with fixed sizes of 1, 2, 4, 8, 16, 32, 64 and 128.
32 is as high as I can push it without the instruction count starting to creep
up again, and gets almost all the block-reduction win of 64 and 128.

…f Vec

This function is responsible for 8.5% of all heap allocation (calls) in CL.
This change avoids almost all of them by using a SmallVec::<[Value; 32]>
instead.  Dynamic instruction count falls by 0.25%.  The fixed size of 32 was
arrived at after profiling with fixed sizes of 1, 2, 4, 8, 16, 32, 64 and 128.
32 is as high as I can push it without the instruction count starting to creep
up again, and gets almost all the block-reduction win of 64 and 128.
Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good, but how frequent is it to have so many branch arguments? Do you have an actual distribution of the values here? 32 seems very high and unlikely in the general case...

@lars-t-hansen
Copy link
Collaborator

I would expect that for large functions, esp as results from aggressive inlining, 32 is not uncommon. And with the new splitter, the number of ebb parameters will increase. My experience with split-across-calls was actually that the increase was significant. If we create profiling code for this it would be nice either to check it in or to stash it someplace where it can be found. Indeed, we keep talking about adding telemetry to check our assumptions in the various compilers. We have to be careful and summarize/bucket/anonymize any data we gather or we're probably doing something that looks like a fingerprint, but it can be done.

Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Makes sense. Thanks!

@bnjbvr bnjbvr merged commit c72ab98 into bytecodealliance:master Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants