Add --trace-dispatch
#55848
Add --trace-dispatch
#55848
Conversation
kpamnany
force-pushed
the
kp/add-trace-dispatch
branch
from
e138bc3
to
8728e7e
Compare
| } | ||
| else { | ||
| if (ios_file(&f_dispatch, t, 1, 1, 1, 1) == NULL) | ||
| jl_errorf("cannot open dispatch statement file \"%s\" for writing", t); |
There was a problem hiding this comment.
Very awkward (and slightly unsound) to throw errors from here. We should try to move this into the jloptions file so we can detect any issues earlier
There was a problem hiding this comment.
I essentially duplicated record_precompile_statement from right above this, which does the same thing.
I added this; does that look right? Is there anywhere else where this should be recorded? |
Similar to `--trace-compile`, emit the `precompile` statement for a method once, but only when it is dynamically dispatched. For this, we add a `dispatched` field into `jl_method_instance_t` that is initialized to 0, and when the method is dispatched, it is set to 1 and the precompile statement is emitted (once).
- Rename `precompiled` field in `jl_method_instance_t` to `flags` and use bit 0 as `precompiled` and bit 1 as `dispatched`. - Use relaxed atomic operations to access `flags`. - Add setting of `dispatched` to `jl_gf_invoke_by_method`.
Also add a test for `--trace-compile`.
kpamnany
force-pushed
the
kp/add-trace-dispatch
branch
from
e3fab0a
to
00bc708
Compare
|
Added a test. |
kpamnany
added
the
status:merge me
src/gf.c
Outdated
| @@ -3360,6 +3396,13 @@ JL_DLLEXPORT jl_value_t *jl_apply_generic(jl_value_t *F, jl_value_t **args, uint | |||
| jl_int32hash_fast(jl_return_address()), | |||
| world); | |||
| JL_GC_PROMISE_ROOTED(mfunc); | |||
| uint8_t miflags = jl_atomic_load_relaxed(&mfunc->flags); | |||
There was a problem hiding this comment.
Are we sure this has no overhead? Could it go in a slow path?
There was a problem hiding this comment.
Good point, not sure. I think the cost should be negligible. Thoughts @vtjnash?
Could add an outer check for jloptions.trace_dispatch? Remove the check from inside the record... function?
There was a problem hiding this comment.
I've moved this atomic load off the fast path.
There was a problem hiding this comment.
This is probably better but I think it should go in a slower lookup path in jl_lookup_generic; at least only when the fast cache misses.
There was a problem hiding this comment.
Ah, I see. Going by the comments in jl_lookup_generic_, there is a lookup in an associative cache, and if that misses, a lookup in the full cache. Can a method instance be in either of those caches if it hasn't been dispatched? If not, then I could move the call here?
There was a problem hiding this comment.
I've moved it here; please take a look?
kpamnany
removed
the
status:merge me
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Just sharing a thought that if the user wants both Maybe that's overthinking it but at least it's possible if desired. |
|
Merge? @JeffBezanson |
Similar to
--trace-compile, emit theprecompilestatement for a method once, but only when it is dynamically dispatched.For this, we add a
dispatchedfield intojl_method_instance_tthat is initialized to 0, and when the method is dispatched, it is set to 1 and the precompile statement is emitted (once).TODO: