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

Add --trace-dispatch #183

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Add --trace-dispatch #183

merged 1 commit into from
Sep 30, 2024

Conversation

kpamnany
Copy link
Collaborator

@kpamnany kpamnany commented Sep 21, 2024

PR Description

Add a --trace-dispatch command line argument similar to --trace-compile; the output will similarly contain precompile statements, but for methods that are dynamically dispatched to, i.e. generically called.

Checklist

Requirements for merging:

@github-actions github-actions bot added port-to-v1.10 This change should apply to Julia v1.10 builds port-to-master This change should apply to all future Julia builds labels Sep 21, 2024
@kpamnany kpamnany removed the port-to-master This change should apply to all future Julia builds label Sep 23, 2024
Similar to `--trace-compile`, emit the `precompile` statement for a method
once, but only when it is dynamically dispatched.

For this, we rename the `precompiled` field in `jl_method_instance_t` to
`flags` and use bit 0 as `precompiled` and bit 1 as `dispatched`.

When the method is dispatched, the `dispatched` bit is set to 1 and the
precompile statement is emitted. This check is done in
`jl_gf_invoke_by_method` and in the slow path (cache miss) of
`jl_apply_generic`.
@kpamnany kpamnany marked this pull request as ready for review September 27, 2024 19:45
Comment on lines +2390 to +2396
if (!jl_has_free_typevars(mi->specTypes)) {
jl_printf(s_dispatch, "precompile(");
jl_static_show(s_dispatch, mi->specTypes);
jl_printf(s_dispatch, ")\n");
if (s_dispatch != JL_STDERR)
ios_flush(&f_dispatch);
}
Copy link
Member

Choose a reason for hiding this comment

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

This is so that we only write out concrete signatures, not ones compiled by inference?

Can you add a comment indicating so? (Also in the upstream PR 🙏 )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, this is just a direct copy from record_precompile_statement. I think jl_static_show will barf if there are free type variables.

Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

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

LGTM. Great stuff here. I'd love to wait for the upstream PR, but if you need it now, you need it.

❤️ ❤️ ❤️

@kpamnany
Copy link
Collaborator Author

Thanks... hoping I get the okay from Jeff today.

@kpamnany kpamnany merged commit 75cf8dc into v1.10.2+RAI Sep 30, 2024
3 checks passed
@kpamnany kpamnany deleted the kp-add-trace-dispatch branch September 30, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port-to-v1.10 This change should apply to Julia v1.10 builds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants