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

show line info + module in ADD_METHOD profiling #49495

Merged
merged 2 commits into from
Apr 27, 2023
Merged

Conversation

KristofferC
Copy link
Sponsor Member

Makes it easier to see where the method is defined:

image

It also acts as a bit of disambguation between different methods that end up having the same description due to the truncation of the full method signature.

@topolarity
Copy link
Member

Adding the file and line number here is a good idea 👌

Using jl_timing_show instead of jl_timing_show_func_sig un-quiets the type printing. Is that intentional?

@KristofferC
Copy link
Sponsor Member Author

Using jl_timing_show instead of jl_timing_show_func_sig un-quiets the type printing. Is that intentional?

I don't understand this. The only difference should be the addition of the file info. It used to call jl_timing_show, now it calls jl_timing_show + jl_timing_printf with the linfo.

@topolarity
Copy link
Member

Ah sorry, you're right. I never quieted the types here, but they are quieted for method instance printing, etc.

@KristofferC KristofferC added the merge me PR is reviewed. Merge when all tests are passing label Apr 26, 2023
@KristofferC KristofferC merged commit bc2fa50 into master Apr 27, 2023
@KristofferC KristofferC deleted the kc/linfo_add_method branch April 27, 2023 05:52
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants