Skip to content

Fast validation#1204

Merged
kripken merged 8 commits intomasterfrom
fast-validation
Oct 2, 2017
Merged

Fast validation#1204
kripken merged 8 commits intomasterfrom
fast-validation

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Sep 29, 2017

This makes wasm validation parallel (the function part). This makes loading+validating tanks (a 12MB wasm file) 2.3x faster on a 4-core machine (from 3.5 to 1.5 seconds). It's a big speedup because most of loading+validating was actually validating.

It's also noticeable during compilation, since we validate by default at the end. 8% faster on -O2 and 23% on -O0. So actually fairly significant on -O0 builds.

As a bonus, this PR also moves the code from being 99% in the header to be 1% in the header, which I think @dschuff will appreciate ;)

@binji
Copy link
Copy Markdown
Member

binji commented Sep 29, 2017

This is a bit surprising to me, since wabt's validation is actually one of the faster parts of loading. I understand binaryen's validation is different, but I assumed it was still single pass, right?

I annotated wasm2wat w/ timings when loading tanks:

read file: 0.0144852s
read binary: 0.684837s
validate: 0.190429s
apply names: 0.116855s
write wat: 1.28688s

These probably could be a lot faster actually... :-} But the point is that validation is actually pretty small.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Sep 29, 2017

Yeah, this is a little weird I guess. Binaryen does some fast validation during reading, like a wasm reader would, but most of the work is done after reading, on the IR. And we validate some internal IR details (like unreachable types being correct, node types not being stale, nodes appearing only once in the IR, etc.), so it's maybe more like LLVM's validation than wasm binary validation in that respect. And those haven't been much optimized - I think there are some places where it might not be linear. So it's both doing more work and not doing it super-efficiently ;)

For now, parallelizing is pretty easy to do and gives a big speedup. We should also probably give an option to not validate, like I think LLVM does.

And thanks for those numbers, that's interesting. Seems like binaryen loading is in the right ballpark but still kind of slow. Btw, is all that single-threaded in wabt?

cc @yurydelendik - @binji's wabt numbers on wasm loading might interest you as you were measuring related things I think.

@binji
Copy link
Copy Markdown
Member

binji commented Sep 29, 2017

Yeah, all single-threaded for now. It's also worth noting that when I enable expression folding it gets quite a bit slower (unoptimized, so probably could improve). I assume binaryen is doing something like this when creating the IR (what I call "read binary" here), so maybe that explains some additional speed differences.

read file: 0.0145758s
read binary: 0.672801s
validate: 0.158918s
apply names: 0.0924712s
write wat: 2.66662s

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Sep 29, 2017

What do you mean by expression folding?

@binji
Copy link
Copy Markdown
Member

binji commented Sep 29, 2017

Turning stuff like this:

i32.const 0
i32.const 1
i32.add

into this:

(i32.add (i32.const 0) (i32.const 1))

That's what the spec calls it, anyway: https://webassembly.github.io/spec/text/instructions.html#folded-instructions

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Sep 29, 2017

I see, yeah. Makes sense that takes more work.

@kripken kripken mentioned this pull request Sep 29, 2017
Comment thread src/wasm-validator.h
void validateMemBytes(uint8_t bytes, WasmType type, Expression* curr);
void validateBinaryenIR(Module& wasm);
struct WasmValidator {
bool validate(Module& module, bool validateWeb = false, bool validateGlobally = true, bool quiet = false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Three bool args make me think this should be flags.

enum ValidatorFlags {
  ValidateWeb = 1 << 0,
  ValidateGlobally = 1 << 1,
  ValidateQuiet = 1 << 2
}
//...
bool WasmValidator::validate(Module& module, ValidatorFlags flags);
//...
validator.flags(wasm, ValidateGlobally | ValidateQuiet);

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.

Yeah, makes sense. How about if I do that in a followup? (I'm trying to change the API as little as possible in this PR)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds reasonable

@kripken kripken merged commit 1f8d8a5 into master Oct 2, 2017
@kripken kripken deleted the fast-validation branch October 2, 2017 20:52
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.

3 participants