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

[WIP] Add jl_start_tracking_dispatches / jl_finish_tracking_dispatches #33710

Closed

Conversation

NHDaly
Copy link
Member

@NHDaly NHDaly commented Oct 28, 2019

Allow users to track all dynamic dispatches that occur when tracking is enabled.

This can be useful for debugging, such as to understand where your code has type-instabilities that lead to a dispatch.

It will also be used by StagedFunctions.jl (and ideally base Generated functions) to cheaply record the set of MethodInstances that need to have backedges set to the generated function. Merging this into #32774 should be enough to fix all the tests! :)

There is an example of this working, in the StagedFunctions package, here:
NHDaly/StagedFunctions.jl#9

Allows users to track all dynamic dispatches that occur when tracking is
enabled.

This can be useful for debugging purposes, such as to
understand where your code has type-instabilities that lead to a
dispatch.

It will also be used by StagedFunctions.jl to cheaply record the
set of MethodInstances that need to have backedges set to the generated
function.
@Keno
Copy link
Member

Keno commented Oct 29, 2019

I'm a little worried about adding extra instructions in a path that's this hot, even with the unlikely annotation. If it's just for debugging I would have suggested a patch point, but sounds like you're imagining a case where generated functions would turn this on automatically?

@JeffBezanson
Copy link
Member

I have to agree it does not seem acceptable to add code to this path.

@NHDaly
Copy link
Member Author

NHDaly commented Oct 30, 2019

Yeah, i had the same concern. :( Thanks for the quick feedback, i was going to ask that same question myself. ❤️

Are there any good alternatives that jump to your mind(s)?

@NHDaly
Copy link
Member Author

NHDaly commented Nov 6, 2019

For completeness, I was trying to measure how much of an impact this would have, but I haven't found a benchmark that actually spends a lot of cpu in jl_invoke. I thought something like this might, but it looks like it's only spending 2.5% of time in jl_invoke according to the Profile:

r(n) = n == 0 ? 0 : Core._apply(r, n-1)

using PProf, Profile

@profile r(1) # warmup r() and profile

Profile.clear()
@profile for _ in 1:10_000; r(1000); end

pprof(from_c=true)

This is showing 2.3% cpu actually in the invoke frame (after this change):
Screen Shot 2019-11-06 at 5 26 13 PM

Can you recommend a better micro-benchmark that would make invoke hotter?

And/or, is there a different spot i could put this check that would catch "all dynamic dispatches"?

EDIT: Actually, it seems to vary between like 2.3% - 3.5%.

@NHDaly
Copy link
Member Author

NHDaly commented Nov 6, 2019

(I bumped it up to more like 3% - 5% when i added more methods to r, to avoid it somehow inlining and avoiding the dispatch.)

@vtjnash
Copy link
Member

vtjnash commented Nov 7, 2019

Usually just something like Base.inferencebarrier(==)(1, 1)

On my machine, this takes about 11.4 nanoseconds, which is probably about 36 cycles or 650 flops (2.7GHz base frequency, BLAS.set_num_threads(1); time * peakflops())

@vtjnash
Copy link
Member

vtjnash commented Nov 22, 2024

replaced by --trace-dispatch

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 this pull request may close these issues.

4 participants