Skip to content

Conversation

timholy
Copy link
Member

@timholy timholy commented Jan 31, 2022

Motivated by JuliaLang/julia#43990 (comment),
but probably not a bad idea in its own right.

Motivated by JuliaLang/julia#43990 (comment),
but probably not a bad idea in its own right.
@codecov
Copy link

codecov bot commented Jan 31, 2022

Codecov Report

Merging #28 (c52b1f2) into master (72ad73e) will decrease coverage by 0.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
- Coverage   75.67%   75.45%   -0.23%     
==========================================
  Files           2        2              
  Lines         111      110       -1     
==========================================
- Hits           84       83       -1     
  Misses         27       27              
Impacted Files Coverage Δ
src/Preferences.jl 73.56% <ø> (-2.23%) ⬇️
src/utils.jl 82.60% <100.00%> (+7.60%) ⬆️

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 72ad73e...c52b1f2. Read the comment docs.

@giordano
Copy link
Member

With Julia v1.7.1, on master (same as v1.2.3):

% julia --project=/tmp --startup-file=no -q
julia> @time using GTK3_jll
  0.104612 seconds (367.33 k allocations: 17.839 MiB, 24.74% compilation time)

(same as JuliaPackaging/JLLWrappers.jl#38 (review))

This PR:

% julia --project=/tmp --startup-file=no -q
julia> @time using GTK3_jll
  0.126395 seconds (415.97 k allocations: 22.106 MiB, 20.87% compilation time)

This seems to go in the wrong direction?

@timholy
Copy link
Member Author

timholy commented Jan 31, 2022

Hmm, I can confirm that. It seems like the final precompilation is bad news. Try the latest?

@timholy
Copy link
Member Author

timholy commented Jan 31, 2022

Checked, commenting out that last one doesn't eat into the benefits for JuliaLang/julia#43990, if anything slightly improved.

@giordano
Copy link
Member

giordano commented Feb 1, 2022

Now it looks better:

% julia --project=/tmp --startup-file=no -q
julia> @time using GTK3_jll
  0.088641 seconds (271.73 k allocations: 15.071 MiB, 29.69% compilation time)

For reference, I got this timing after cleaning up the .julia/compiled directory. I did the same again before rerunning the benchmark for Preferences.j master:

% julia --project=/tmp --startup-file=no -q
julia> @time using GTK3_jll
  0.088586 seconds (270.22 k allocations: 14.794 MiB, 29.98% compilation time)

Memory allocations got just slightly worse, but the time is pretty much the same

@timholy
Copy link
Member Author

timholy commented Feb 1, 2022

I think this is even better. Hold off on merging, though, the hasmethod line may change.

@timholy
Copy link
Member Author

timholy commented Feb 24, 2022

Now that JuliaLang/julia#43990 has merged without further API changes, we can safely merge this.

@timholy timholy merged commit 4b01be6 into master Feb 24, 2022
@timholy timholy deleted the teh/latency branch February 24, 2022 10: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.

2 participants