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

Expose lprofGetLoadModuleSignature #26

Open
daxpedda opened this issue Dec 17, 2024 · 5 comments · May be fixed by #27
Open

Expose lprofGetLoadModuleSignature #26

daxpedda opened this issue Dec 17, 2024 · 5 comments · May be fixed by #27

Comments

@daxpedda
Copy link

daxpedda commented Dec 17, 2024

It would be very useful if we could mimic the filenames generated by LLVM with minicov. While we can't really include InstrProfilingFile.c, at least lprofGetLoadModuleSignature is already there. With that the rest could be done independently of minicov.

If this is in scope, I would be happy to make the PR myself!

@Amanieu
Copy link
Owner

Amanieu commented Dec 17, 2024

Sure, I think it's reasonable to expose the module signature.

Could you also explain the workflow that requires matching LLVM's file names? I personally just dump everything to a single file and then pass it to grcov.

@daxpedda
Copy link
Author

Could you also explain the workflow that requires matching LLVM's file names? I personally just dump everything to a single file and then pass it to grcov.

I'm currently working on adding nextest support to wasm-bindgen-test-runner, which executes all tests in parallel. Therefor the runner process is spawned many times and I can't merge everything in a single file. So while there are a bunch of possible solutions to that, my overall efforts so far have been to align as close as possible to cargo test, which solves this issue by producing unique file names for every file.

So no, my workflow doesn't not require the module signature, but it would be great to have and the tradeoff seems quite small to me.

@Amanieu
Copy link
Owner

Amanieu commented Dec 17, 2024

If you don't actually care about the name, just that it's unique, then there are better ways of doing this than using lprofGetLoadModuleSignature. Is there a specific reason for wanting the same names as the ones generated by LLVM?

@daxpedda
Copy link
Author

If you don't actually care about the name, just that it's unique, then there are better ways of doing this than using lprofGetLoadModuleSignature.

Indeed, I'm aware!

Is there a specific reason for wanting the same names as the ones generated by LLVM?

wasm-bindgen is a foundational and low-level crate. I'm trying to eliminate any differences to cargo test to allow users to expect the same output as close as I can possibly get it. In this case I'm trying to emulate LLVM_PROFILE_FILE. wasm-bindgen-test-runner is able to cover all the patterns except %m. However, instead of %m generating the instrumented binary signature, I can just generate a random number or a hash of the output. I would however prefer simply aligning with LLVM if possible.

But no, there is no specific use-case that I anticipate that will require files to be named exactly the same.

@Amanieu
Copy link
Owner

Amanieu commented Dec 17, 2024

I think it's still fine to expose this from minicov.

@daxpedda daxpedda linked a pull request Dec 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants