Closed ranocha closed 2 years ago
Merging #513 (21d7fd5) into master (ceb2b65) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #513 +/- ##
=======================================
Coverage 87.10% 87.10%
=======================================
Files 28 28
Lines 1846 1846
=======================================
Hits 1608 1608
Misses 238 238
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
I'll merge it. How does this work?
Here is an example when this test passes: https://github.com/SciML/MuladdMacro.jl/runs/8121563802?check_suite_focus=true
And here is a failing example: https://github.com/ranocha/InvalidationsTest.jl/runs/8134758303?check_suite_focus=true
Other than that, it will just add a new CI run (that is relatively cheap). It will fail when a PR introduces new invalidations (and also report the numbers of invalidations for each PR). From there on, you can decide whether you want to ignore it or chase down the invalidations & fix them.
Do you have any specific questions about this?
For example, this package and its dependencies currently cause 500 invalidations: https://github.com/JuliaMath/Interpolations.jl/runs/8136468587?check_suite_focus=true#step:9:1
I guess I'll have to run SnoopCompiler myself to figure which 500?
Yes, exactly. Fixing invalidations is quite a manual process in my experience anyway, so you need to run things locally to figure out what's going on and how to fix things. The docs of SnoopCompile.jl are a good place to learn about this: https://timholy.github.io/SnoopCompile.jl/stable/snoopr/
This is based on https://github.com/julia-actions/julia-invalidations. Adding such checks came up in https://discourse.julialang.org/t/potential-performance-regressions-in-julia-1-8-for-special-un-precompiled-type-dispatches-and-how-to-fix-them/86359. I suggest to add this check here since this package is widely used as a dependency.
See also SciML/MuladdMacro.jl#26 and SciML/MuladdMacro.jl#29