Skip to content

Conversation

@quinnj
Copy link
Member

@quinnj quinnj commented Aug 24, 2021

Been wanting/needing to do this for a while. Worked through the
excellent tutorials in
https://timholy.github.io/SnoopCompile.jl/stable/snoopi_deep_parcel/ to
help figure out cases where we were overspecializing certain methods
(CSV.Context) and generating useful precompile statements. On my
machine, this reduces the time-to-first-read (TTFR) from ~8s to ~1.5,
while package loading time goes from 3s to 10s, which both seem like
good tradeoffs. To avoid overspecialization of CSV.Context method,
which has a huge method body, we introduce a simple Arg wrapper struct
to kill inference and make the method signature concrete. The @refargs
macro conveniently wraps each arg in a call with Arg(x), and for a
definition, it annotates each arg as x::Arg and inserts an unwrap as
the first thing in the method body like x = x[]::Int where x was
declared as x::Int in the argument list.

One thing I noticed that I would still like to look into if time is that if we do f = CSV.File(IOBuffer("a,b,c\n1,2,3\n\n")) in a fresh session, it takes ~1.5s, but then if we call like f = CSV.File(IOBuffer("a,b,c\n1,2,3\n\n"); header=false), it takes 3s to......recompile a bunch of stuff I guess? I'd love to figure out why that's causing so much recompiling.

Been wanting/needing to do this for a while. Worked through the
excellent tutorials in
https://timholy.github.io/SnoopCompile.jl/stable/snoopi_deep_parcel/ to
help figure out cases where we were overspecializing certain methods
(`CSV.Context`) and generating useful precompile statements. On my
machine, this reduces the time-to-first-read (TTFR) from ~8s to ~1.5,
while package loading time goes from 3s to 10s, which both seem like
good tradeoffs. To avoid overspecialization of `CSV.Context` method,
which has a huge method body, we introduce a simple `Arg` wrapper struct
to kill inference and make the method signature concrete. The `@refargs`
macro conveniently wraps each arg in a _call_ with `Arg(x)`, and for a
_definition_, it annotates each arg as `x::Arg` and inserts an unwrap as
the first thing in the method body like `x = x[]::Int` where `x` was
declared as `x::Int` in the argument list.
@codecov
Copy link

codecov bot commented Aug 24, 2021

Codecov Report

Merging #875 (8e4f4a9) into main (462f40e) will decrease coverage by 1.57%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #875      +/-   ##
==========================================
- Coverage   90.77%   89.19%   -1.58%     
==========================================
  Files           9        9              
  Lines        1961     2008      +47     
==========================================
+ Hits         1780     1791      +11     
- Misses        181      217      +36     
Impacted Files Coverage Δ
src/utils.jl 83.58% <ø> (+0.11%) ⬆️
src/chunks.jl 91.30% <100.00%> (ø)
src/context.jl 84.27% <100.00%> (-4.04%) ⬇️
src/detection.jl 92.28% <100.00%> (-4.38%) ⬇️
src/file.jl 95.16% <100.00%> (-1.08%) ⬇️
src/precompile.jl 96.66% <100.00%> (+46.66%) ⬆️
src/rows.jl 86.54% <100.00%> (-0.08%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 462f40e...8e4f4a9. Read the comment docs.

@quinnj quinnj merged commit 4191215 into main Aug 24, 2021
@quinnj quinnj deleted the jq/snooping branch August 24, 2021 20:10
@rikhuijzer
Copy link

this reduces the time-to-first-read (TTFR) from ~8s to ~1.5,

Awesome! ❤️

@timholy
Copy link
Contributor

timholy commented Aug 28, 2021

Nice work, @quinnj! With reference to https://discourse.julialang.org/t/csv-jl-fails-precompling-typeerror-in-type-expression-expected-unionall-got-type-parsers-options/67195, one thing I've noticed (but not yet updated the SnoopCompile docs for, sorry), is that to more easily support multiple Julia versions & architectures (e.g., 32-bit vs 64-bit), often a better strategy is to have your precompile file look like this:

function _precompile_()
    ccall(:jl_generating_output, Cint, ()) == 1 || return nothing
    x = my_pkg_object(args...)
    do_some_work(x, otherargs...)
end

for some tiny workloads typical of the data types you want your package to work on. This has the same effect as explicit precompile directives, except it's more thorough because any runtime-dispatches will also get precompiled without a separate precompile directive. And it better supports different Julia versions and architectures because it allows the compilation to proceed in the natural way on each machine.

In other words, rather than creating a workload for snooping via @snoopi_deep and then using parcel + write, better is to just paste your workload into the precompile file itself!

@timholy
Copy link
Contributor

timholy commented Aug 28, 2021

It's the same as precompile because precompile is not a directive to "save my code in the cache": it simply forces the specialization to be compiled. Julia saves everything that's "owned" by the package in the cache automatically, whether it was compiled due to a precompile directive or simply by running code.

@rikhuijzer
Copy link

Very nice @timholy. Now, I get what you meant when we were talking about it during JuliaCon. I did get it, but couldn't find any example anywhere. Looking forward to docs too 👍

@timholy
Copy link
Contributor

timholy commented Aug 28, 2021

Yeah, the problem with voluminous docs is that when you realize new stuff it's a bit daunting to update them. I really need to restructure it in more cookbook format ("do this...").

quinnj added a commit that referenced this pull request Aug 29, 2021
The changes in files that are not precompile.jl are inference
improvements; mainly from inspecting results of `@code_typed`,
Cthulhu.jl, and SnoopCompile.jl. The changes in precompile.jl are from
comments from @timholy recommending that in our precompile process, we
can just call regular code instead needing to call `precompile` with
methods/arg types. I'm aware I don't understand all the details around
precompilation, method invalidation, etc. but unfortunately, I feel a
bit blocked with CSV.jl's precompilation. With the changes in #875, we
now see a fixed overhead of allocations when parsing due, I'm told, to
an issue in Base Julia
(JuliaLang/julia#34055).
quinnj added a commit that referenced this pull request Sep 1, 2021
* A little refactoring to improve inference and precompilation

The changes in files that are not precompile.jl are inference
improvements; mainly from inspecting results of `@code_typed`,
Cthulhu.jl, and SnoopCompile.jl. The changes in precompile.jl are from
comments from @timholy recommending that in our precompile process, we
can just call regular code instead needing to call `precompile` with
methods/arg types. I'm aware I don't understand all the details around
precompilation, method invalidation, etc. but unfortunately, I feel a
bit blocked with CSV.jl's precompilation. With the changes in #875, we
now see a fixed overhead of allocations when parsing due, I'm told, to
an issue in Base Julia
(JuliaLang/julia#34055).
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