wasmtime / issue #4944 Don't enable parallel compilation ... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #4944 Don't enable parallel compilation ...

Wasmtime GitHub notifications bot (Sep 22 2022 at 22:49):

Hmm something is enabling rayon by default even with this. cargo doesn't provide great visibility into what is turning on what features here.

Wasmtime GitHub notifications bot (Sep 23 2022 at 00:28):

jameysharp commented on issue #4944:

I was just noticing earlier that --no-default-features isn't turning off rayon any more. Thanks for prompting me to dig deeper.

I think the cause is that #4911 made wasmtime-cli-flags depend on the wasmtime/parallel-compilation feature.

I'm very much in favor of disabling parallel compilation by default for benchmarking, so let's figure out how to get the web of features right.

You can get cargo to tell you what's pulling in a particular dependency with e.g. cargo tree -p wasmtime-bench-api -i rayon. It also accepts the usual --no-default-features and --features flags so you can check different combinations.

Wasmtime GitHub notifications bot (Sep 23 2022 at 15:39):

abrown commented on issue #4944:

@fitzgen, if a runtime flag is needed, I had submitted #4207 for that purpose (along with an --engine-flags flag in Sightglass, IIRC). Looks like something went wrong with #4207 but I can rebase that and figure out the CI error if that would help--what do you think? In the past I've also needed to benchmark single-threaded compilation but I think I used the taskset hammer instead, so I'm in favor of making either of these approaches work.

Wasmtime GitHub notifications bot (Sep 23 2022 at 16:32):

jameysharp commented on issue #4944:

If I'm understanding #4207 correctly, it looks like it's worth merging but isn't necessary for non-WASI options like the recent --disable-parallel-compilation.

Wasmtime GitHub notifications bot (Sep 23 2022 at 17:51):

fitzgen commented on issue #4944:

I think we don't want to rely on only a runtime flag because the stacks are still not great when profiling if we do the runtime flag.

I think we will want to make the CLI flags crate not enable a bunch of features and maybe make Config always have methods for things like disabling parallelism even when the rayon dep isn't active (but document that it has no effect in that case or make it panic or return an error or something).

Wasmtime GitHub notifications bot (Sep 23 2022 at 17:54):

abrown commented on issue #4944:

If I'm understanding https://github.com/bytecodealliance/wasmtime/pull/4207 correctly, it looks like it's worth merging but isn't necessary for non-WASI options like the recent --disable-parallel-compilation.

No, it actually makes it possible to use any of the CommonOptions, of which disable_parallel_compilation is one.

Wasmtime GitHub notifications bot (Sep 23 2022 at 17:55):

alexcrichton commented on issue #4944:

because the stacks are still not great when profiling

Could you expand on this? The flag is checked here so this shouldn't affect stacks other than having a run_maybe_parallel frame which otherwise would probably get inlined if rayon is disabled (but whether or not the frame is present doesn't seem like a bad stack to me)

Wasmtime GitHub notifications bot (Sep 23 2022 at 17:58):

abrown commented on issue #4944:

the stacks are still not great when profiling if we do the runtime flag

I agree with that sentiment (not wanting to wade through rayon stacks) but I'm wondering how you're still seeing large stacks when you enable the flags: I see self.config().parallel_compilation as guarding against using rayon at all in a few places. Do you mean the rayon "setup the environment" functions?

Wasmtime GitHub notifications bot (Sep 23 2022 at 18:09):

fitzgen commented on issue #4944:

I guess the broken stacks that I'm seeing now are unrelated to rayon, its all in vec iter stuff but it isn't all the same so the flamegraphs and top down views such are kinda unusable. I guess we can close this PR tho.

Last updated: Oct 23 2024 at 20:03 UTC