-
Notifications
You must be signed in to change notification settings - Fork 34
Improve precompilation #359
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
base: master
Are you sure you want to change the base?
Conversation
| deps = PkgId[PkgId(UUID(v), k) for (k, v) in get(prj, "deps", Dict{String,Any}())] | ||
| weakdeps = | ||
| PkgId[PkgId(UUID(v), k) for (k, v) in get(prj, "weakdeps", Dict{String,Any}())] | ||
| deps::Vector{PkgId} = PkgId[PkgId(UUID(v), k) for (k::String, v::String) in get(prj, "deps", Dict())] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaFormatter] reported by reviewdog 🐶
| deps::Vector{PkgId} = PkgId[PkgId(UUID(v), k) for (k::String, v::String) in get(prj, "deps", Dict())] | |
| deps::Vector{PkgId} = | |
| PkgId[PkgId(UUID(v), k) for (k::String, v::String) in get(prj, "deps", Dict())] |
| name = "Aqua" | ||
| uuid = "4c88cf16-eb10-579e-8560-4a9242c79595" | ||
| authors = ["Takafumi Arakaki <[email protected]> and contributors"] | ||
| version = "1.0.0-DEV" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaProjectFormatter] reported by reviewdog 🐶
| version = "1.0.0-DEV" |
| authors = ["Takafumi Arakaki <[email protected]> and contributors"] | ||
| version = "1.0.0-DEV" | ||
| authors = ["Takafumi Arakaki <[email protected]> and contributors"] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaProjectFormatter] reported by reviewdog 🐶
| version = "1.0.0-DEV" | |
124a83f to
3883d38
Compare
|
I fixed some of the formatting issues, but I felt the other suggestions make the code less readable 🤷 |
| write(project_file, """ | ||
| name = "AquaFakePackage" | ||
| uuid = "5a23b2e7-8c45-4b1c-9d3f-7a6b4c8d9e0f" | ||
| version = "0.1.0" | ||
| [deps] | ||
| Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" | ||
| [compat] | ||
| Test = "1" | ||
| julia = "1" | ||
| """) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaFormatter] reported by reviewdog 🐶
| write(project_file, """ | |
| name = "AquaFakePackage" | |
| uuid = "5a23b2e7-8c45-4b1c-9d3f-7a6b4c8d9e0f" | |
| version = "0.1.0" | |
| [deps] | |
| Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" | |
| [compat] | |
| Test = "1" | |
| julia = "1" | |
| """) | |
| write( | |
| project_file, | |
| """ | |
| name = "AquaFakePackage" | |
| uuid = "5a23b2e7-8c45-4b1c-9d3f-7a6b4c8d9e0f" | |
| version = "0.1.0" | |
| [deps] | |
| Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" | |
| [compat] | |
| Test = "1" | |
| julia = "1" | |
| """, | |
| ) |
In particular to `find_stale_deps()`, which took about 600ms to compile.
3883d38 to
e08666e
Compare
| project_extras = false, | ||
| stale_deps = false, | ||
| deps_compat = false, | ||
| persistent_tasks = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaFormatter] reported by reviewdog 🐶
| persistent_tasks = false) | |
| persistent_tasks = false, | |
| ) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #359 +/- ##
==========================================
+ Coverage 86.77% 87.07% +0.30%
==========================================
Files 11 12 +1
Lines 514 534 +20
==========================================
+ Hits 446 465 +19
- Misses 68 69 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # Create a minimal fake package to test | ||
| module _FakePackage | ||
| export fake_function | ||
| fake_function() = 1 | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not putting this inside the @setup_workload instead of saving a useless module in the compile cache?
In an effort to bring down the TTFX/runtime. On ZMQ.jl this brings the time for
Aqua.test_all(ZMQ)down from ~8.7s to ~6.1s, so about 30%. The downside is that the precompilation time increased from ~1.4s to ~11s, which is quite a hit. My feeling is that it's still worth it though since Aqua has relatively few dependencies and is unlikely to get invalidated frequently. I'd recommend reviewing each commit separately.