-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Wasmtime's profiling support #6361
Refactor Wasmtime's profiling support #6361
Conversation
This commit is a bit of reorganization around profiling-related code in Wasmtime with the aim of eventually simplifying it a bit more. The changes here are: * All exposed agents are removed and instead only constructor functions returning trait objects are now exposed. * All `*_disabled.rs` files and modules are removed in favor of a function that returns a result (less trait impls). * All `*_linux.rs` files where renamed to just `*.rs`. (less files in play) * The `pid` and `tid` arguments were removed as they were only used by the jitdump profiler and now that manages it internally. * Registering an entire ELF image is now part of the trait rather than buried within the trampoline code in Wasmtime.
In general Wasmtime's support for DWARF is not great so this is rarely used and at least in my experience this hasn't been necessary to get good information from perf. This commit removes the processing here which while probably useful is probably not necessary and otherwise makes the jidump agent a bit of an odd-one-out relative among the other agents.
Refactor slightly to account for this.
This has been the same as `self.pid` for quite some time but with `rustix` it's pretty easy to get access to the current thread id.
Add a second argument to registration of an entire module for custom names to get functions named correctly, and otherwise profilers now only need to look at individual functions.
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:config"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
To modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
This commit fixes a panic that's easy to hit with native profilers by accident. This was introduced in bytecodealliance#6262 which made it into the 9.0.0 release but is not present on `main` due to the refactorings of bytecodealliance#6361 which are on `main` but not on 9.0.0. Closes bytecodealliance#6433
This commit refactors the profiling support for existing agents in Wasmtime. The end goal was to handle #6328 and I ended up doing a fair bit of other cleanup along the way. The profilers are now all centered around "load a single function" rather than dealing with both modules and trampolines. The shared logic of parsing an ELF file is located in one place now in shared support across all profilers.
This also applies refactorings such as removing DWARF support for jitdump which while not problematic hasn't been necessary for me historically. Additionally Wasmtime's support for DWARF is too buggy to enable so it's not getting any use anyway.