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 module_roots #157

Closed
wants to merge 1 commit into from
Closed

Add module_roots #157

wants to merge 1 commit into from

Conversation

timholy
Copy link
Owner

@timholy timholy commented Nov 25, 2020

module_roots makes it easier to discover precompile-worthy
MethodInstances. The promise of @snoopi_deep is that it is not
restricted to just the entrance calls to inference; consequently,
we can find the most-expensive-to-infer calls in a specific module and
precompile those calls.

We may want to rewrite the entire parcel infrastructure around this, but perhaps one step at a time. We might want to create some kind of aggregate report that combines analysis of excessive specialization, inference breaks, and an improved set of precompile statements.

`module_roots` makes it easier to discover precompile-worthy
MethodInstances. The promise of `@snoopi_deep` is that it is not
restricted to just the entrance calls to inference; consequently,
we can find the most-expensive-to-infer calls in a specific module and
precompile those calls.
@timholy
Copy link
Owner Author

timholy commented Nov 30, 2020

@NHDaly, if we were to generate some sort of "latency report and amelioration plan" for users & developers, here are the pieces I'm envisioning in very rough terms:

  • for methods that get a huge number of specializations that lead to long compilation times, recommend that users try to figure out a way to streamline them through the approximate relationship compilation_time ∝ method_complexity * number_of_specializations, and that decreasing either factor can help. This is mostly what Add accumulate_by_method #152 was about.
  • for methods that have a reasonably small number of specializations and where the types they're being called with are intrinsic to the package or its dependencies, generate precompile directives. That's mostly what this PR is about---with @snoopi_deep we can sometimes find something good to precompile even if it's not the entrance point to inference.
  • for methods with big latencies that have relatively few specializations but which can't plausibly be precompiled in this package, recommend mechanisms to get them precompiled in downstream packages. This requires that methods be sufficiently type-stable that they can be inferred across package boundaries, as dump.c only stashes MethodInstances that either correspond to a method defined in the package or that have a backedge to the package. Identifying "breaks" in the chain of inference, where Julia uses runtime dispatch, will enable precompilation of bigger chunks of code. That's basically what Add inference_triggers #159 is about.

When you get a chance, I would love a review on this PR, and subsequently #159, so we can get the API moving towards completion.

@timholy
Copy link
Owner Author

timholy commented Dec 13, 2020

I'm just going to start merging these, likely after taking another pass at reworking them.

@timholy
Copy link
Owner Author

timholy commented Dec 14, 2020

I suspect this was superseded by #168, but I'll leave it here for a while at least.

Copy link

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

Fantastic documentation additions! :)

@timholy
Copy link
Owner Author

timholy commented Dec 14, 2020

But also almost certainly unnecessary! #168 has more powerful functionality. We need to add docs, but I am still feeling my way forward.

If you've not seen it, check out the new "visualizations.jl" and runtime_inferencetime. The aim is to get PGO, but statically rather than dynamically. There are advantages to either, but we can do the static one today...and it might actually be a better fit for the Julia mindset.

@timholy
Copy link
Owner Author

timholy commented Dec 26, 2020

Yeah, this isn't really necessary after #168.

@timholy timholy closed this Dec 26, 2020
@timholy timholy deleted the teh/module_roots branch December 26, 2020 13:53
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.

2 participants