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 a copy-paste invalidations analysis script #326

Merged
merged 10 commits into from
Jan 3, 2023

Conversation

ChrisRackauckas
Copy link
Contributor

As much as details are nice, sometimes giving something that's easy to copy/repeat is good. Maybe this could be added as a library function (it probably would need to be a macro).

As much as details are nice, sometimes giving something that's easy to copy/repeat is good. Maybe this could be added as a library function (it probably would need to be a macro).
## A copy-paste analysis of invalidations

The following is a quick "grab and go" script for analyzing invalidations. Insert your code into the `@snoopr` block. The resulting plot shows the
distributions of the invalidations sorted by the number of children effected. Generally, invalidations with many children matter more than those
Copy link

Choose a reason for hiding this comment

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

effected -> affected

Copy link
Owner

@timholy timholy left a comment

Choose a reason for hiding this comment

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

This looks like a good idea. I made some specific recommendations. However, I think the recipe in https://discourse.julialang.org/t/julia-v1-9-0-beta2-is-fast/92290/17?u=tim.holy has several merits, most particularly that by using precompile_blockers you focus just on the invalidations that affect your workload. (I tend to fix invalidations wherever I find them, but I think for many developers they don't want to be worried about, say, REPL invalidations when their goal is to reduce TTFX for their package.)

To turn it into a more fleshed-out example of my schematic recipe, you could put the using OrdinaryDiffEq and method definition under @snoopr (those are the parts that can cause invalidation) and the rest (the parts that may trigger inference) under @snoopi_deep and then use precompile_blockers.

docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Outdated Show resolved Hide resolved
@ChrisRackauckas
Copy link
Contributor Author

I'll go by your recommendations, but I think my main points are:

  1. I think the tutorial should have a simple copy paste "what are my invalidations?" before the deeper explanation of the tool
  2. It should also produce a plot to show the relative scale of which invalidations matter.

So I updated to make use of the version from https://timholy.github.io/SnoopCompile.jl/dev/reference/#SnoopCompile.precompile_blockers. For some reason I find that way it's written to be much easier to read than the one in https://discourse.julialang.org/t/julia-v1-9-0-beta2-is-fast/92290/17?u=tim.holy, maybe just due to white space.

For the plots, should I be doing that all on staletrees instead of trees?

@ChrisRackauckas
Copy link
Contributor Author

Maybe this should be in a separate "Getting Started" or "SnoopCompiler 101 for Inexperienced Programmers" or something. I think the big thing about SnoopCompile is that now with the precompilation changes and all, it needs to move from an audience of "I know a bit of how Julia works and want to make a good package better" to "this is some standard Julia 201 stuff like type-stability". I don't think your average "just started developing packages" developer should need to know how to dig into the details and fix invalidations. I think it's sufficient for many to just know that (a) invalidations are an issue, (b) here's how I create a report of invalidations and open an issue. If people are opening these issues and the big ones are brought into the conversation, then I think they will be solved.

@timholy
Copy link
Owner

timholy commented Jan 1, 2023

I agree this has to get a lot simpler. Most of the docs were written in the course of this basically being a research project, trying to understand what factors would affect precompilation. Now that Julia itself is much more capable, this is a much less complex question than it used to be. We definitely need to make the documentation reflect the new world.

In fact what I think we also desperately need is a way to take the analysis that Cthulhu does, but instead of annotating the type-inferred code, instead annotates the raw source text. E.g.,

    for (i, x::Any) in enumerate(v)
        g[i] = f(x)::Any
    end

but making that happen is, I think, quite difficult. JuliaLang/julia#31162 would make this vastly easier, but I don't know lisp and have no intention of learning.

Copy link
Owner

@timholy timholy left a comment

Choose a reason for hiding this comment

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

Just a couple more things to think about and then I think we can merge this.

docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Show resolved Hide resolved
docs/src/tutorial.md Outdated Show resolved Hide resolved
show(trees[end]) # show the most invalidating method

# Count number of children (number of invalidations per invalidated method)
n_invalidations = map(trees) do methinvs
Copy link
Owner

Choose a reason for hiding this comment

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

So I guess the question is, do we want to plot all invalidations or just the ones that affect the workload? I can go with this, I just worry it might seem daunting.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about plotting both next to each other?

Copy link
Owner

Choose a reason for hiding this comment

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

Or in different colors on the same axis.

docs/src/tutorial.md Outdated Show resolved Hide resolved
@timholy
Copy link
Owner

timholy commented Jan 3, 2023

I think we can go with this, and add other forms of plotting later. Thanks @ChrisRackauckas! I'll merge when CI finishes.

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Base: 84.08% // Head: 77.04% // Decreases project coverage by -7.04% ⚠️

Coverage data is based on head (7a6aa05) compared to base (dcae62d).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
- Coverage   84.08%   77.04%   -7.05%     
==========================================
  Files          17       17              
  Lines        2162     2021     -141     
==========================================
- Hits         1818     1557     -261     
- Misses        344      464     +120     
Impacted Files Coverage Δ
src/invalidation_and_inference.jl 0.00% <0.00%> (-89.66%) ⬇️
src/invalidations.jl 63.90% <0.00%> (-13.90%) ⬇️
src/parcel_snoopi_deep.jl 86.32% <0.00%> (-3.58%) ⬇️
SnoopPrecompile/src/SnoopPrecompile.jl 85.71% <0.00%> (-1.79%) ⬇️
src/write.jl 90.90% <0.00%> (-1.40%) ⬇️
src/deep_demos.jl 57.89% <0.00%> (-1.08%) ⬇️
src/parcel_snoopl.jl 89.47% <0.00%> (-0.53%) ⬇️
src/parcel_snoopi.jl 93.83% <0.00%> (-0.31%) ⬇️
src/visualizations.jl 0.00% <0.00%> (ø)
src/parcel_snoopc.jl 78.37% <0.00%> (+0.22%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@timholy timholy merged commit 95a3b41 into timholy:master Jan 3, 2023
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