JuliaIO / JSON.jl

JSON parsing and printing
Other
311 stars 100 forks source link

add basic precompilation #337

Closed rafaqz closed 2 years ago

rafaqz commented 2 years ago

This tiny PR Improves the ttfx time of Blink.jl by 5 seconds. Probably also helps other packages that use JSON.jl

Current master here and with Blink.jl:

julia> @time using Blink
  5.583824 seconds (17.56 M allocations: 740.126 MiB, 6.44% gc time, 88.95% compilation time)

julia> @time Window()
 12.164188 seconds (28.92 M allocations: 1.293 GiB, 3.17% gc time, 98.19% compilation time)
Window(1, Electron(Process(`/home/raf/.julia/dev/Blink/deps/atom/electron /home/raf/.julia/dev/Blink/src/AtomShell/main.js port 3466`, ProcessRunning), Sockets.TCPSocket(RawFD(22) active, 0 bytes waiting), Dict{String, Any}("callback" => Blink.var"#1#2"())), Page(1, WebSocket(server, CONNECTED), Dict{String, Any}("webio" => Blink.AtomShell.var"#22#23"{Blink.AtomShell.WebIOBlinkComm}(Blink.AtomShell.WebIOBlinkComm(Window(#= circular reference @-5 =#))), "callback" => Blink.var"#1#2"()), Distributed.Future(1, 1, 1, Some(true))), Task (done) @0x00007fdcd5809cd0)

This PR:

julia> @time using Blink
  5.477089 seconds (17.55 M allocations: 739.919 MiB, 6.47% gc time, 88.74% compilation time)

julia> @time Window()
  6.973419 seconds (14.42 M allocations: 779.433 MiB, 3.20% gc time, 97.07% compilation time)
Window(1, Electron(Process(`/home/raf/.julia/dev/Blink/deps/atom/electron /home/raf/.julia/dev/Blink/src/AtomShell/main.js port 4505`, ProcessRunning), Sockets.TCPSocket(RawFD(22) active, 0 bytes waiting), Dict{String, Any}("callback" => Blink.var"#1#2"())), Page(1, WebSocket(server, CONNECTED), Dict{String, Any}("webio" => Blink.AtomShell.var"#22#23"{Blink.AtomShell.WebIOBlinkComm}(Blink.AtomShell.WebIOBlinkComm(Window(#= circular reference @-5 =#))), "callback" => Blink.var"#1#2"()), Distributed.Future(1, 1, 1, Some(true))), Task (done) @0x00007f462344ab30)
codecov[bot] commented 2 years ago

Codecov Report

Merging #337 (40a2d28) into master (f93688a) will decrease coverage by 0.87%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
- Coverage   99.51%   98.63%   -0.88%     
==========================================
  Files           5        5              
  Lines         410      441      +31     
==========================================
+ Hits          408      435      +27     
- Misses          2        6       +4     
Impacted Files Coverage Δ
src/JSON.jl 20.00% <0.00%> (-80.00%) :arrow_down:
src/Parser.jl 100.00% <0.00%> (ø)
src/specialized.jl 100.00% <0.00%> (ø)
src/Writer.jl 98.26% <0.00%> (+0.13%) :arrow_up:

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 f93688a...40a2d28. Read the comment docs.

KristofferC commented 2 years ago

That's crazy. What is it that takes so much time to compile? Must be Parsers.jl?

Edit: Before https://github.com/JuliaIO/JSON.jl/pull/285 _precompile_() took 0.25 seconds, now it takes 4 seconds. 😬

rafaqz commented 2 years ago

I have another PR over at Parser.jl that has a similar crazy effect on CSV.jl https://github.com/JuliaData/Parsers.jl/pull/108

Something going on...

rafaqz commented 2 years ago

The combined effect of both of these PRs on Blink.jl


julia> @time using Blink
  2.005740 seconds (3.79 M allocations: 255.856 MiB, 7.74% gc time, 68.09% compilation time)

julia> @time Window()
  6.817974 seconds (14.42 M allocations: 779.489 MiB, 3.63% gc time, 97.04% compilation time)
Window(1, Electron(Process(`/home/raf/.julia/dev/Blink/deps/atom/electron /home/raf/.julia/dev/Blink/src/AtomShell/main.js port 5005`, ProcessRunning), Sockets.TCPSocket(RawFD(22) active, 0 bytes waiting), Dict{String, Any}("callback" => Blink.var"#1#2"())), Page(1, WebSocket(server, CONNECTED), Dict{String, Any}("webio" => Blink.AtomShell.var"#22#23"{Blink.AtomShell.WebIOBlinkComm}(Blink.AtomShell.WebIOBlinkComm(Window(#= circular reference @-5 =#))), "callback" => Blink.var"#1#2"()), Distributed.Future(1, 1, 1, Some(true))), Task (done) @0x00007f1e570f9cd0)

So precompilation here is still catching something not covered in Parsers.jl precompilation.

rafaqz commented 2 years ago

Can we get this merged?

KristofferC commented 2 years ago

Tests fail due to the _precompile_() in the test. Not sure why it is there at all.

rafaqz commented 2 years ago

Oh right I pushed that after asking to merge :sweat_smile:

Was just looking to get rid of missing code coverage lines. What do you think?

KristofferC commented 2 years ago

It runs during precompilation so it is tested. Imo just remove it.