Skip to content

Optimize wasm reading#1202

Merged
kripken merged 2 commits intomasterfrom
opt-bin-read
Sep 28, 2017
Merged

Optimize wasm reading#1202
kripken merged 2 commits intomasterfrom
opt-bin-read

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Sep 27, 2017

Use a set of the breaks we've seen, don't rescan blocks to see if they have breaks to them.

This makes wasm loading 25% faster, ignoring validation.

Side note: Validation is by far most of the time spent when loading wasm. Perhaps we should have a --no-validation option?

…an blocks to see if they have breaks to them
@dschuff
Copy link
Copy Markdown
Member

dschuff commented Sep 27, 2017

When do you want to load but don't care about validation?

Comment thread src/wasm-binary.h Outdated
BreakTarget(Name name, int arity) : name(name), arity(arity) {}
};
std::vector<BreakTarget> breakStack;
std::unordered_set<Name> seenBreaks;
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.

How about calling this breakTargets instead, since it's not a set of the break instructions, but rather their targets?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, maybe, but we have a struct called BreakTarget so that might be confusing. It contains a name and and arity. So maybe breakTargetNames?

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.

Sounds good.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Sep 27, 2017

Well, maybe if someone is sure it's valid binary that was emitted as part of a build system, maybe they would want to disable validation. Or maybe we should optimize validation speed ;) It's not even parallelized currently.

@kripken kripken merged commit ee9d515 into master Sep 28, 2017
@kripken kripken deleted the opt-bin-read branch September 28, 2017 18:07
@kripken
Copy link
Copy Markdown
Member Author

kripken commented Sep 28, 2017

Parallelizing validation is probably worth it too since it's noticeable time in compilation, not just when reading (we validate at the end by default, currently). I'll look into it.

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.

2 participants