Skip to content

Commit

Permalink
disable precompilation for selected packages via Preferences (#334)
Browse files Browse the repository at this point in the history
Co-authored-by: Tim Holy <tim.holy@gmail.com>
  • Loading branch information
t-bltg and timholy authored Jan 9, 2023
1 parent 333b645 commit fa775b3
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 3 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/Documenter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ defaults:
run:
shell: bash

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
Documenter:
name: Documentation
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ on:
defaults:
run:
shell: bash
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
jobs:
test:
if: "!contains(github.event.head_commit.message, '[skip ci]')"
Expand Down
4 changes: 4 additions & 0 deletions SnoopPrecompile/Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ uuid = "66db9d55-30c0-4569-8b51-7e840670fc0c"
authors = ["Tim Holy <tim.holy@gmail.com> and contributors"]
version = "1.0.2"

[deps]
Preferences = "21216c6a-2e73-6563-6e65-726566657250"

[compat]
Preferences = "1"
julia = "1"

[extras]
Expand Down
12 changes: 12 additions & 0 deletions SnoopPrecompile/src/SnoopPrecompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ module SnoopPrecompile

export @precompile_all_calls, @precompile_setup

@static if VERSION >= v"1.6"
using Preferences
end

@static if VERSION >= v"1.6"
const skip_precompile = @load_preference("skip_precompile", String[])
else
const skip_precompile = String[]
end

const verbose = Ref(false) # if true, prints all the precompiles
const have_inference_tracking = isdefined(Core.Compiler, :__set_measure_typeinf)
const have_force_compile = isdefined(Base, :Experimental) && isdefined(Base.Experimental, Symbol("#@force_compile"))
Expand Down Expand Up @@ -44,6 +54,7 @@ end
- indirect runtime-dispatched calls to such methods.
"""
macro precompile_all_calls(ex::Expr)
string(__module__) in skip_precompile && return :()
if have_force_compile
ex = quote
begin
Expand Down Expand Up @@ -101,6 +112,7 @@ runtime dispatches (though they will be precompiled anyway if the runtime-callee
to your package).
"""
macro precompile_setup(ex::Expr)
string(__module__) in skip_precompile && return :()
return esc(quote
# let
if ccall(:jl_generating_output, Cint, ()) == 1 || $SnoopPrecompile.verbose[]
Expand Down
7 changes: 7 additions & 0 deletions SnoopPrecompile/test/SnoopPC_D/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name = "SnoopPC_D"
uuid = "8c347477-c4a5-4b7f-b911-c68b2b9ed891"
authors = ["Tim Holy <tim.holy@gmail.com>"]
version = "0.1.0"

[deps]
SnoopPrecompile = "66db9d55-30c0-4569-8b51-7e840670fc0c"
11 changes: 11 additions & 0 deletions SnoopPrecompile/test/SnoopPC_D/src/SnoopPC_D.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module SnoopPC_D

using SnoopPrecompile

@precompile_setup begin
@precompile_all_calls begin
global workload_ran = true
end
end

end # module SnoopPC_D
20 changes: 17 additions & 3 deletions SnoopPrecompile/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ using UUIDs
push!(LOAD_PATH, @__DIR__)

using SnoopPC_A
if Base.VERSION >= v"1.8.0-rc1"
if VERSION >= v"1.8.0-rc1"
# Check that calls inside :setup are not precompiled
m = which(Tuple{typeof(Base.vect), Vararg{T}} where T)
have_mytype = false
Expand Down Expand Up @@ -39,7 +39,7 @@ using UUIDs
@test count == 1
end

if Base.VERSION >= v"1.7" # so we can use redirect_stderr(f, ::Pipe)
if VERSION >= v"1.7" # so we can use redirect_stderr(f, ::Pipe)
pipe = Pipe()
id = Base.PkgId(UUID("d38b61e7-59a2-4ef9-b4d3-320bdc69b817"), "SnoopPC_B")
redirect_stderr(pipe) do
Expand All @@ -50,7 +50,21 @@ using UUIDs
@test occursin(r"UndefVarError: `?missing_function`? not defined", str)
end

if Base.VERSION >= v"1.6"
if VERSION >= v"1.6"
using SnoopPC_C
end

if VERSION >= v"1.6"
script = """
push!(LOAD_PATH, @__DIR__)
using SnoopPC_D
exit(isdefined(SnoopPC_D, :workload_ran) === parse(Bool, ARGS[1]) ? 0 : 1)
"""

SnoopPrecompile.Preferences.set_preferences!(SnoopPrecompile, "skip_precompile" => ["SnoopPC_D"])
@test success(run(`$(Base.julia_cmd()) --project=$(Base.active_project()) -e $script 0`))

SnoopPrecompile.Preferences.delete_preferences!(SnoopPrecompile, "skip_precompile"; force = true)
@test success(run(`$(Base.julia_cmd()) --project=$(Base.active_project()) -e $script 1`))
end
end
11 changes: 11 additions & 0 deletions docs/src/snoop_pc.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,14 @@ intended calls from inside the `@precompile_all_calls` block (see [`@snoopi_deep
code that you want precompiled.
You can use `@snoopi_deep` to check for any (re)inference when you use the code in your package.
To fix any specific problems, you can combine `@precompile_all_calls` with manual `precompile` directives.

One can reduce the cost of precompilation for selected packages using the `Preferences.jl` based mechanism and the `skip_precompile` key:
```julia
using SnoopPrecompile, Preferences
set_preferences!(SnoopPrecompile, "skip_precompile" => ["PackageA", "PackageB"])
```

After restarting julia, the `@precompile_all_calls` and `@precompile_setup` workloads will be disabled (locally) for `PackageA` and `PackageB`.

!!! note
Changing `skip_precompile` may result in a one-time recompilation of all packages that use SnoopPrecompile.

8 comments on commit fa775b3

@timholy
Copy link
Owner

Choose a reason for hiding this comment

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

@nanosoldier runtests(subdir="SnoopPrecompile", vs="@333b645e256ed09d7e785a3a6beef81f863b85e7")

@nanosoldier
Copy link

Choose a reason for hiding this comment

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

Your job was not accepted, please verify the trigger phrase syntax.

@timholy
Copy link
Owner

Choose a reason for hiding this comment

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

@maleadt, any suggestions for how to do this better? See https://github.com/timholy/SnoopCompile.jl/commits/master, this is attempting to compare the merge of #334 against the previous commit.

@maleadt
Copy link

Choose a reason for hiding this comment

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

I currently did not implement a vs mode when testing packages; it's always comparing against the latest released version. I figured that it wouldn't be common to compare two unreleased versions of a package, and to compare against released-but-not-latest versions of a package is not straightforward (we currently just register a new version and let Pkg resolve, as there isn't an easy way to fix a dependency to a specific version).

I guess I could just register the vs commit too though... Let me investigate.

@timholy
Copy link
Owner

Choose a reason for hiding this comment

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

Ah, great! So that previous run that you did probably already tested this commit? (It was merged before you triggered the run.)

Note that SnoopPrecompile has an independent release process and is not included in SnoopCompile itself. I.e., SnoopCompile v2.9.8 does not include SnoopPrecompile at all. SnoopPrecompile is at v1.0.1.

@timholy
Copy link
Owner

Choose a reason for hiding this comment

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

I'll just go ahead and release. Thanks @maleadt!

@maleadt
Copy link

Choose a reason for hiding this comment

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

So that previous run that you did probably already tested this commit? (It was merged before you triggered the run.)

Yes, it included that.

I've added support for the vs keyword, and also made runtests on PRs behave similarly to doing the same on the Julia repo: now it'll compare against the merge-head (typically the master branch) instead of just using the latest released version from the registry. That should be more accurate, as it'll only test changes from the PR. If you want to test against a released version, you can always point vs to a tag.

I'll deploy this shortly.

@timholy
Copy link
Owner

Choose a reason for hiding this comment

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

Sounds awesome! Thanks for your outstanding stewardship, it really means a lot to the whole community.

Please sign in to comment.