wasmtime / PR #9185 vtune: smoke test profiling in CI · git-wasmtime

One thing to note here: I've tried to only install VTune where needed (when we check wasmtime-cli) because it increases the build time by at least a minute. I know we've worked in the past to reduce build times so there's a trade-off to think about: faster builds or test more features?

Wasmtime GitHub notifications bot (Aug 30 2024 at 14:52):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2024 at 14:52):

alexcrichton created PR review comment:

To confirm, does this test fail if this flag is left out?

Wasmtime GitHub notifications bot (Aug 30 2024 at 14:53):

alexcrichton commented on PR #9185:

The integration path here seems like a reasonable starting point to me, thanks! I think it's fine to start here and see how it goes

Wasmtime GitHub notifications bot (Sep 04 2024 at 00:30):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Sep 04 2024 at 00:30):

abrown created PR review comment:

No, one can always run vanilla Wasmtime in VTune and that should be valid whether this flag is present or not. What this flag does is inform VTune about the JIT-compiled code so it can display it appropriately in the UI. We've had problems in the past with incorrectly telling VTune about this code which _did_ result in errors and that is what this test is checking: in effect, this is a regression test to check that we correctly tell VTune about the JIT code.

Wasmtime GitHub notifications bot (Sep 04 2024 at 00:34):

abrown commented on PR #9185:

The integration path here seems like a reasonable starting point to me, thanks! I think it's fine to start here and see how it goes

I worked on the install-vtune-action a bit more and v2 can now install VTune in 20 seconds using a ~700MB cache (see here). I haven't switched to that version for this PR because I'm not sure if we can afford caches that large; just saying it's available.

Wasmtime GitHub notifications bot (Sep 04 2024 at 14:48):

alexcrichton submitted PR review:

Ok sounds good to me :+1:. I think things run fast enough here we can eat the extra minute for vtune, but if that becomes an issue we can investigate caching in the future.

Wasmtime GitHub notifications bot (Sep 05 2024 at 20:13):

abrown updated PR #9185.

Wasmtime GitHub notifications bot (Sep 06 2024 at 17:33):

abrown commented on PR #9185:

Not sure what is going on with CI here: with the latest matrix.filter == 'linux-x64' change, VTune no longer installs in the riscv64 and s390x runs, but somehow the check that runs vtune --version is returning successfully (it should not!) and the full test attempts to run. Not sure what is going on: is the vtune --version check incorrect somehow or is there some GitHub runner cached state causing issues here?

Wasmtime GitHub notifications bot (Sep 06 2024 at 17:34):

abrown edited a comment on PR #9185:

Not sure what is going on with CI: with the latest matrix.filter == 'linux-x64' change, VTune no longer installs in the riscv64 and s390x runs, but somehow the check that runs vtune --version is returning successfully (it should not!) and the full test attempts to run. Is the vtune --version check incorrect somehow or is there some GitHub runner cached state causing issues here?

Wasmtime GitHub notifications bot (Sep 06 2024 at 17:34):

abrown edited a comment on PR #9185:

Not sure what is going on with CI: with the latest matrix.filter == 'linux-x64' change, VTune no longer installs in the riscv64 and s390x runs, but somehow the check that runs vtune --version is returning successfully (it should not!) and the full test attempts to run. Is the vtune --version check incorrect somehow or is there some GitHub runner cached state causing issues here? [edit: let's try a rebase...]

Wasmtime GitHub notifications bot (Sep 06 2024 at 17:35):

abrown updated PR #9185.

Wasmtime GitHub notifications bot (Sep 06 2024 at 17:43):

alexcrichton commented on PR #9185:

Ah I think the issue is that this needs to be skipped in qemu. Otherwise it's running a non-x64 binary in vtune which isn't going to work. We don't have a great "am I in qemu" test other than #[cfg(target_arch = "...")] so running this test only on x64 should be fine

Wasmtime GitHub notifications bot (Sep 06 2024 at 17:49):

abrown updated PR #9185.

Wasmtime GitHub notifications bot (Sep 06 2024 at 18:26):

abrown merged PR #9185.

Last updated: Nov 22 2024 at 16:03 UTC