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

rework precompilation using SnoopPrecompile #4334

Merged
merged 6 commits into from
Sep 20, 2022
Merged

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Sep 4, 2022

Fix #4079.

  1. rework examples structure (mostly syntax, split imports, saves a non neglictible amount of lines);
  2. use SnoopPrecompile to generate precompilation statements from Plots examples;
  3. drop the SnoopCompile workflow (broken on 1.8+), and the (noisy) precompilation related auto PRs;
  4. bump GR to 0.68.

Blocking issue with SnoopPrecompile and GR: timholy/SnoopCompile.jl#295.

GR fix: jheinen/GR.jl#465.

@jheinen
Copy link
Member

jheinen commented Sep 16, 2022

@t-bltg : Please let me know when it's finished. I will then merge the GR development branch in the master branch and tag a new version ASAP.

@t-bltg
Copy link
Member Author

t-bltg commented Sep 16, 2022

@t-bltg : Please let me know when it's finished.

I've reworked it, and this should be ready for review / comments.

@t-bltg t-bltg marked this pull request as ready for review September 16, 2022 13:25
@t-bltg t-bltg changed the title rework precompilation rework precompilation using SnoopPrecompile Sep 16, 2022
@BeastyBlacksmith
Copy link
Member

Did you test locally if this precompiles with the GR branch? I would also be interested to see the differences in loading times/ TTFP.

@t-bltg
Copy link
Member Author

t-bltg commented Sep 19, 2022

Did you test locally if this precompiles with the GR branch

jheinen/GR.jl#465 is needed for this PR to work, so we are awaiting a new GR version from @jheinen.

I would also be interested to see the differences in loading times/ TTFP.

Scenario, with GR@master, on 1.8.1:

main() = begin
  for i in (1, 5, 10, 11, 13, 14, 15, 16, 19, 20, 23, 24, 27, 28, 35, 38, 53, 54, 56, 57, 58, 60)
    script = """
    using Plots
    ex(i) = begin
      pl = Plots.test_examples(:gr, i, disp=false)
      Plots.png(pl, "gr-" * string(i))
      Plots.pdf(pl, "gr-" * string(i))
    end

    @time ex(parse(Int, first(ARGS)))
    """
    run(`$(Base.julia_cmd()) -e $script $i`)
  end
end

main()

[email protected]

[ Info: Testing plot: gr:1:Lines
 10.276396 seconds (10.68 M allocations: 554.345 MiB, 4.33% gc time, 99.41% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:5:Global
 13.367969 seconds (16.11 M allocations: 831.746 MiB, 4.16% gc time, 99.36% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:10:Histogram2D
 12.630873 seconds (14.24 M allocations: 744.904 MiB, 4.18% gc time, 99.49% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:11:Line types
 11.810301 seconds (14.16 M allocations: 736.157 MiB, 4.27% gc time, 99.15% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:13:Marker types
 10.646469 seconds (11.89 M allocations: 617.070 MiB, 4.00% gc time, 98.97% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:14:Bar
 11.272947 seconds (13.10 M allocations: 679.028 MiB, 4.43% gc time, 99.45% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:15:Histogram
 11.688254 seconds (12.62 M allocations: 656.876 MiB, 3.99% gc time, 99.49% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:16:Subplots
 10.518674 seconds (12.41 M allocations: 649.709 MiB, 4.16% gc time, 99.29% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:19:Open/High/Low/Close
 10.089114 seconds (10.40 M allocations: 540.604 MiB, 3.94% gc time, 99.48% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:20:Annotations
 11.637137 seconds (13.55 M allocations: 705.220 MiB, 4.11% gc time, 99.33% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:23:Pie
 10.939707 seconds (13.37 M allocations: 695.601 MiB, 4.35% gc time, 99.33% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:24:3D
 12.762531 seconds (15.16 M allocations: 790.374 MiB, 4.12% gc time, 99.30% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:27:Polar Plots
 10.341857 seconds (10.80 M allocations: 560.419 MiB, 3.84% gc time, 99.28% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:28:Heatmap, categorical axes, and aspect_ratio
 11.085066 seconds (14.73 M allocations: 750.466 MiB, 5.07% gc time, 99.41% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:35:Lines and markers with varying colors
 11.759844 seconds (13.21 M allocations: 684.206 MiB, 3.93% gc time, 99.23% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:38:Histogram2D (complex values)
 12.849030 seconds (14.66 M allocations: 766.957 MiB, 4.09% gc time, 99.45% compilation time: 5% of which was recompilation)
[ Info: Testing plot: gr:53:Step Types
 10.968680 seconds (12.48 M allocations: 649.566 MiB, 3.99% gc time, 99.22% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:54:Guide positions and alignment
 11.958675 seconds (14.41 M allocations: 746.445 MiB, 4.10% gc time, 99.12% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:56:Bar plot customizations
 14.026715 seconds (16.85 M allocations: 878.849 MiB, 3.71% gc time, 99.35% compilation time: 3% of which was recompilation)
[ Info: Testing plot: gr:57:Vertical and horizonal spans
 10.837500 seconds (12.35 M allocations: 642.858 MiB, 4.36% gc time, 99.41% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:58:Stacked area chart
 11.005754 seconds (13.02 M allocations: 677.565 MiB, 4.37% gc time, 99.38% compilation time: 4% of which was recompilation)
[ Info: Testing plot: gr:60:3D projection
 11.738911 seconds (14.53 M allocations: 759.607 MiB, 4.54% gc time, 99.11% compilation time: 4% of which was recompilation)

Plots@PR

[ Info: Testing plot: gr:1:Lines
  7.817280 seconds (765.89 k allocations: 38.379 MiB, 2.45% gc time, 96.77% compilation time: 19% of which was recompilation)
[ Info: Testing plot: gr:5:Global
 10.675643 seconds (5.27 M allocations: 273.975 MiB, 2.98% gc time, 99.25% compilation time: 16% of which was recompilation)
[ Info: Testing plot: gr:10:Histogram2D
 10.268749 seconds (4.96 M allocations: 259.561 MiB, 3.43% gc time, 99.36% compilation time: 17% of which was recompilation)
[ Info: Testing plot: gr:11:Line types
  9.338404 seconds (1.77 M allocations: 92.299 MiB, 1.91% gc time, 99.14% compilation time: 16% of which was recompilation)
[ Info: Testing plot: gr:13:Marker types
  8.407991 seconds (1.52 M allocations: 77.188 MiB, 98.57% compilation time: 18% of which was recompilation)
[ Info: Testing plot: gr:14:Bar
  8.883329 seconds (2.25 M allocations: 116.019 MiB, 99.18% compilation time: 17% of which was recompilation)
[ Info: Testing plot: gr:15:Histogram
  9.327184 seconds (2.06 M allocations: 106.498 MiB, 99.27% compilation time: 16% of which was recompilation)
[ Info: Testing plot: gr:16:Subplots
  8.792240 seconds (2.63 M allocations: 137.007 MiB, 3.07% gc time, 99.06% compilation time: 6% of which was recompilation)
[ Info: Testing plot: gr:19:Open/High/Low/Close
  8.084275 seconds (783.65 k allocations: 39.444 MiB, 2.41% gc time, 96.84% compilation time: 19% of which was recompilation)
[ Info: Testing plot: gr:20:Annotations
  9.189509 seconds (1.65 M allocations: 85.196 MiB, 2.84% gc time, 99.12% compilation time: 16% of which was recompilation)
[ Info: Testing plot: gr:23:Pie
  8.664856 seconds (2.02 M allocations: 104.369 MiB, 3.08% gc time, 96.75% compilation time: 18% of which was recompilation)
[ Info: Testing plot: gr:24:3D
 10.682581 seconds (6.28 M allocations: 332.592 MiB, 2.98% gc time, 99.19% compilation time: 14% of which was recompilation)
[ Info: Testing plot: gr:27:Polar Plots
  8.879845 seconds (2.61 M allocations: 133.232 MiB, 3.07% gc time, 99.09% compilation time: 17% of which was recompilation)
[ Info: Testing plot: gr:28:Heatmap, categorical axes, and aspect_ratio
  9.261369 seconds (4.95 M allocations: 258.421 MiB, 3.10% gc time, 99.27% compilation time: 6% of which was recompilation)
[ Info: Testing plot: gr:35:Lines and markers with varying colors
  9.784166 seconds (2.67 M allocations: 137.627 MiB, 2.42% gc time, 98.98% compilation time: 15% of which was recompilation)
[ Info: Testing plot: gr:38:Histogram2D (complex values)
 11.133116 seconds (5.01 M allocations: 262.668 MiB, 2.57% gc time, 99.28% compilation time: 14% of which was recompilation)
[ Info: Testing plot: gr:53:Step Types
  9.069108 seconds (1.62 M allocations: 83.649 MiB, 2.96% gc time, 99.00% compilation time: 17% of which was recompilation)
[ Info: Testing plot: gr:54:Guide positions and alignment
  9.831715 seconds (2.39 M allocations: 124.463 MiB, 2.55% gc time, 98.62% compilation time: 15% of which was recompilation)
[ Info: Testing plot: gr:56:Bar plot customizations
 11.356569 seconds (5.69 M allocations: 297.598 MiB, 2.60% gc time, 99.25% compilation time: 12% of which was recompilation)
[ Info: Testing plot: gr:57:Vertical and horizonal spans
  8.583749 seconds (2.17 M allocations: 113.321 MiB, 99.16% compilation time: 17% of which was recompilation)
[ Info: Testing plot: gr:58:Stacked area chart
  8.759219 seconds (2.12 M allocations: 110.625 MiB, 99.21% compilation time: 17% of which was recompilation)
[ Info: Testing plot: gr:60:3D projection
  9.168367 seconds (3.69 M allocations: 197.162 MiB, 2.79% gc time, 98.88% compilation time: 16% of which was recompilation)

So this PR is actually better for TTFP ↘25% (mostly through reducing allocations significantly - more than 60% and up to 80%).

@BeastyBlacksmith
Copy link
Member

BeastyBlacksmith commented Sep 19, 2022

jheinen/GR.jl#465 is needed for this PR to work

Okay we need to reflect this in the compat section of the Project.toml file then, otherwise this is good to go, given that we release after GR.jl. ( once we solved the macOS timeouts )

@t-bltg
Copy link
Member Author

t-bltg commented Sep 19, 2022

we need to reflect this in the compat section of the Project.toml file

Done in ae44c0c, and awaiting [email protected] release, of course we won't merge this before CI is green.

once we solved the macOS timeouts

Where ?

@BeastyBlacksmith
Copy link
Member

Where ?

On master. Worst case we need to revert #4346

@t-bltg
Copy link
Member Author

t-bltg commented Sep 19, 2022

Mmmh I don't any hang in ci on any OS - especially macOS (https://github.com/JuliaPlots/Plots.jl/actions/runs/3081000582/jobs/4979033686).

But the docs are broken because of PyPlot and conda dependencies: I tried to debug it in PlotDocs and here during the we, but without success (libstdc++ version issue).

@BeastyBlacksmith
Copy link
Member

Mmmh I don't any hang in ci on any OS - especially macOS

Thats good! Might have been fluke then.

@t-bltg
Copy link
Member Author

t-bltg commented Sep 19, 2022

Regarding #4346, I had to comment

# plotattr() # interactive (JLFzf)
in #4356, because when running the tests locally, it displayed a search prompt, making the test wait indefinitely for user input (maybe that was what you meant by hang).

@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Base: 80.81% // Head: 80.53% // Decreases project coverage by -0.27% ⚠️

Coverage data is based on head (86d8170) compared to base (88b646d).
Patch coverage: 70.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4334      +/-   ##
==========================================
- Coverage   80.81%   80.53%   -0.28%     
==========================================
  Files          28       28              
  Lines        7317     7336      +19     
==========================================
- Hits         5913     5908       -5     
- Misses       1404     1428      +24     
Impacted Files Coverage Δ
src/Plots.jl 66.66% <0.00%> (-21.14%) ⬇️
src/examples.jl 91.42% <89.36%> (-5.50%) ⬇️
src/components.jl 86.82% <0.00%> (-2.59%) ⬇️
src/utils.jl 75.39% <0.00%> (-0.16%) ⬇️
src/args.jl 80.22% <0.00%> (-0.15%) ⬇️
src/recipes.jl 67.78% <0.00%> (+0.25%) ⬆️
src/pipeline.jl 94.88% <0.00%> (+0.39%) ⬆️
src/layouts.jl 80.34% <0.00%> (+0.56%) ⬆️

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.

@jheinen
Copy link
Member

jheinen commented Sep 19, 2022

We're still fixing some bugs in the GR run-time - will tag the [email protected] version (hopefully) tomorrow along with a GR_jll bugfix release.

@jheinen
Copy link
Member

jheinen commented Sep 20, 2022

[email protected] is on the way! The new tag should appear in the next few minutes ...

@t-bltg t-bltg merged commit afa4095 into JuliaPlots:master Sep 20, 2022
@jheinen
Copy link
Member

jheinen commented Sep 22, 2022

@t-bltg : I made a change in GR#master (see this commit) to allow conditionally importing the GR_jll package. This patch is required if you want to use a local GR installation instead of the BinaryBuilder artefacts. I hope, this won't break the next release. If you have any concerns or new issues arise, please let me know.

For us, using a local GR run-time is (still) essential.

@t-bltg
Copy link
Member Author

t-bltg commented Sep 22, 2022

@jheinen, this change will break the new precompilation sequence introduced in this PR and now in master (not yet released).

ERROR: LoadError: InitError: Evaluation into the closed module `GR` breaks incremental compilation because the side effects will not be permanent. This is likely due to some other module mutating `GR` with `eval` during precompilation - don't do this.

For us, using a local GR run-time is (still) essential.

I don't understand: what is preventing changing the artifacts using a local installation using Preferences, that will override the GR_jll artifact path ?

@jheinen
Copy link
Member

jheinen commented Sep 22, 2022

I don't understand: what is preventing changing the artifacts using a local installation using Preferences, that will override the GR_jll artifact path ?

Because it's not sufficient only to change GR_jll paths - I have to stop Julia from loading GR_jll at all, because this leads to problems at run-time (probably due to outdated versions or old build systems). My understanding is that I can only overwrite stuff with LocalPreferences.toml or @set_preferences!() - but maybe I just misunderstood.

@t-bltg
Copy link
Member Author

t-bltg commented Sep 22, 2022

@jheinen, let's continue the discussion in jheinen/GR.jl#471.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI continuous integration load / precompilation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Enable precompilation for nightly
3 participants