Skip to content
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

demange names in guest-profiler output #7809

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

high-cloud
Copy link
Contributor

issue #7665

@high-cloud high-cloud requested a review from a team as a code owner January 24, 2024 14:37
@high-cloud high-cloud requested review from alexcrichton and removed request for a team January 24, 2024 14:37
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Jan 24, 2024
Copy link

Subscribe to Label Action

cc @peterhuene

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 .github/subscribe-to-label.json configuration file.

Learn more.

Copy link
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

I hesitated about the use of unwrap, but demangle_function_name can only fail if the writer fails, and I don't think the Write implementation for String can fail, so I'll sign off that this use is fine. We have another unwrap on the use of this function in CompiledModule::register_debug_and_profiling, anyway.

While looking at that, I just noticed that there's a demangle_function_name_or_index function which is very similar to what this whole match statement does. I think it's harmless to duplicate that here, with a slightly different format for the fallback case when there's no function name.

I'm going to merge this as-is. But if you want to open a follow-up PR, I think it would be an improvement to use demangle_function_name_or_index here.

@jameysharp jameysharp added this pull request to the merge queue Jan 24, 2024
Merged via the queue into bytecodealliance:main with commit 5c5640f Jan 24, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants