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

Simplify the API to @precompile_module(M) #5

Merged
merged 9 commits into from
Apr 30, 2022
Merged

Simplify the API to @precompile_module(M) #5

merged 9 commits into from
Apr 30, 2022

Conversation

rikhuijzer
Copy link
Owner

@rikhuijzer rikhuijzer commented Apr 29, 2022

This PR:

  • simplifies the API since package authors now only have to write @precompile_module(Foo).
  • avoids writing to a file (not that this saves time tho xD see below)
  • fixes the following risks for failures:
  1. An include from include(precompile_directives(M)) turns out to be wrong. Developers wouldn't push this, so the risk doesn't lie there. However, this package could make a mistake in generating precompile directives causing breaking downstream packages? Would be better to show warnings in that case and still allow the package to be used.
  2. Other weird things on very weird setups? Probably best is to wrap every precompile call in a function with a try-catch.

Benchmark

On Julia 1.7.3 with Pluto

Before this PR (via include(precompile_directives(Pluto)))

Precompiled in 17 seconds

 Section                        ncalls     time    %tot     alloc    %tot
 ────────────────────────────────────────────────────────────────────────
 import Pluto                        1    1.27s    3.9%    160MiB    5.9%
 compiletimes.jl                     1    31.6s   96.1%   2.49GiB   94.1%
   PlutoRunner.run_expression        1    1.75s    5.3%    137MiB    5.1%
   SessionActions.open               1    23.0s   69.9%   1.80GiB   67.9%
   Pluto.run                         1    1.43s    4.3%    110MiB    4.1%
 ────────────────────────────────────────────────────────────────────────

After this PR (via @precompile_module(Pluto))

Precompiled in 17 seconds

 Section                        ncalls     time    %tot     alloc    %tot
 ────────────────────────────────────────────────────────────────────────
 import Pluto                        1    1.28s    3.9%    157MiB    5.8%
 compiletimes.jl                     1    31.1s   96.1%   2.49GiB   94.2%
   PlutoRunner.run_expression        1    1.74s    5.4%    137MiB    5.0%
   SessionActions.open               1    22.6s   69.8%   1.80GiB   67.9%
   Pluto.run                         1    1.43s    4.4%    110MiB    4.1%
 ────────────────────────────────────────────────────────────────────────

@rikhuijzer rikhuijzer enabled auto-merge (squash) April 29, 2022 20:11
@rikhuijzer rikhuijzer disabled auto-merge April 29, 2022 20:11
@rikhuijzer rikhuijzer changed the title Make the package even safer Make the package safer Apr 29, 2022
@rikhuijzer rikhuijzer enabled auto-merge (squash) April 29, 2022 20:11
@rikhuijzer rikhuijzer disabled auto-merge April 29, 2022 20:12
@rikhuijzer rikhuijzer changed the title Make the package safer Simplify the API to @precompile_module(M) Apr 30, 2022
@rikhuijzer rikhuijzer enabled auto-merge (squash) April 30, 2022 06:29
@rikhuijzer rikhuijzer merged commit 656a6c1 into main Apr 30, 2022
@rikhuijzer rikhuijzer deleted the rh/more-safe branch April 30, 2022 06:33
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.

1 participant