Stream: git-wasmtime

Topic: wasmtime / issue #3687 Support vtune profiling of trampol...

Wasmtime GitHub notifications bot (Jan 13 2022 at 18:32):

github-actions[bot] commented on issue #3687:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jan 13 2022 at 18:41):

alexcrichton commented on issue #3687:

Oh sorry and to clarify, you're right that module.trampolines() is trampolines for everything exported from that module. This is basically a trampoline-per-type-signature for any functions which are "possibly exported", including all those in tables, exported functions, etc.

The trampoline_load() method is only called for trampolines dynamically created through the Func::new API which generates a one-off trampoline specifically for the signature of the function created. Unless Func::new is called somehow (which it ideally shouldn't be because Func::wrap is a faster embedding) then trampoline_load() won't be invoked.

Wasmtime GitHub notifications bot (Jan 17 2022 at 16:28):

bnjbvr commented on issue #3687:

Updated! Also refactored a bit how the loading of dynamic trampolines is done, so that there's less code duplication with the jitdump_linux; if that approach looks good to you too, I could go ahead and do the same for single functions and trampolines for exported signatures?

Wasmtime GitHub notifications bot (Jan 18 2022 at 15:15):

alexcrichton commented on issue #3687:

Looks great to me, thanks! Did you want to do that refactoring as a follow-up or attach here? (either way is fine by me)

Wasmtime GitHub notifications bot (Jan 19 2022 at 14:21):

bnjbvr commented on issue #3687:

Did you want to do that refactoring as a follow-up or attach here? (either way is fine by me)

Can do that here! I wanted to make sure this is something we wanted first :+1:

Wasmtime GitHub notifications bot (Jan 19 2022 at 15:09):

bnjbvr commented on issue #3687:

Ended up giving up on the refactoring (instead of loading a whole module at once, load function per function), because:

• if the profiling agent is the null one, that would have done a bit more work that ends up being wasted (computing the function's pointer, address, default function debug display name, etc.).
• this introduced a bit of infra to generate the default function debug display name on demand
• the vtune profiling agent required a new ProfilingAgent trait method to know that we started loading a new module (so it would know what module display name to generate).

Anyhow, this added more lines of code than there was before, and made the code messier overall, so not a win in my opinion. Spotted a data structure that could be removed, though!

Wasmtime GitHub notifications bot (Jan 19 2022 at 15:49):

alexcrichton commented on issue #3687:

Seems reasonable to me!

Last updated: Nov 22 2024 at 16:03 UTC