Skip to content
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

feat: lake: save elaborated config as an olean #2480

Merged
merged 3 commits into from
Aug 31, 2023

Conversation

tydeu
Copy link
Member

@tydeu tydeu commented Aug 29, 2023

Local benchmarks indicate that this does not have much effect on simple configurations, but has a major impact on complex ones. For instance, using an prebuilt OLean gave a ~5x configuration speedup for the FFI example on my machine. Essentially, it makes configure time near constant, rather than directly proportional to the complexity of the configuration.

The limiting factor for simple configurations appears to be that the initial import of Lake is still expensive.

@tydeu tydeu added the WIP This is work in progress, and only a PR for the sake of CI or sharing. No review yet. label Aug 29, 2023
@tydeu tydeu force-pushed the lakefile-olean branch 3 times, most recently from cfdf918 to b74f52f Compare August 29, 2023 22:47
@tydeu tydeu added awaiting-review Waiting for someone to review the PR and removed WIP This is work in progress, and only a PR for the sake of CI or sharing. No review yet. labels Aug 30, 2023
@tydeu
Copy link
Member Author

tydeu commented Aug 30, 2023

While I would like a review, merging should wait until this is tested against benchmarks like #2457.

@tydeu tydeu added WIP This is work in progress, and only a PR for the sake of CI or sharing. No review yet. and removed awaiting-review Waiting for someone to review the PR labels Aug 31, 2023
@tydeu
Copy link
Member Author

tydeu commented Aug 31, 2023

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit fda7e93.
There were no significant changes against commit a7efe5b.

@tydeu
Copy link
Member Author

tydeu commented Aug 31, 2023

Comparing this benchmark with the plain one in #2457 (comment) displays the expected significant improvement to configuration time. From the comparison:

  Benchmark         Metric         Change
  =================================================
+ lake build no-op  branch-misses  -12.1% (-16.4 σ)
+ lake build no-op  branches       -9.3% (-35.4 σ)
+ lake build no-op  instructions   -7.5% (-28.2 σ)
+ lake configure    branch-misses  -79.5% (-217.3 σ)
+ lake configure    branches       -70.7% (-990.4 σ)
+ lake configure    instructions   -70.9% (-783.7 σ)
+ lake configure    maxrss         -7.5% (-16.5 σ)
+ lake configure    task-clock     -52.4% (-34.7 σ)
+ lake configure    wall-clock     -52.6% (-35σ)

@tydeu tydeu added awaiting-review Waiting for someone to review the PR and removed WIP This is work in progress, and only a PR for the sake of CI or sharing. No review yet. labels Aug 31, 2023
@kim-em
Copy link
Collaborator

kim-em commented Aug 31, 2023

On Mathlib master, lake env currently takes 1.72s.

On the toolchain leanprover/lean4-pr-releases/pr-release-2480:

  • without a lakefile.olean present lake env takes 1.05s, or
  • with a lakefile.olean present lake env takes 0.91s

@tydeu
Copy link
Member Author

tydeu commented Aug 31, 2023

@semorrison

On Mathlib master, lake env currently takes 1.72s.
without a lakefile.olean present lake env takes 1.05s
with a lakefile.olean present lake env takes 0.91s

This is surprising. I am not sure why the configuration without the OLean would have gotten significantly faster. Similarly, I am surprised the OLean only gave an ~10% boost.

When you ran the test without the OLean, did you run it in fresh repository or did you just delete the lakefile.olean? If the latter, the difference may be due to the lakefile.olean files still remaining in dependencies (e.g., proofwidgets). Instead of deleting files, you can use the -R option to force Lake to re-elaborate the configuration from the source.

@kim-em
Copy link
Collaborator

kim-em commented Aug 31, 2023

That was it. Now I get (averaging 10 runs each):

Is the slowdown with -R expected?

@tydeu
Copy link
Member Author

tydeu commented Aug 31, 2023

@semorrison Yes, Lake now just memoizes the import state, not the full environment (because that environment cannot be used by the cached oleans), so there is some expected overhead from recreating it for each dependency.

EDIT: I did not feel it was worth it to also cache the environment on top of that because it would would only be useful for -R, which hopefully will not be commonly used, so the overhead is acceptable.

@tydeu
Copy link
Member Author

tydeu commented Aug 31, 2023

@semorrison One thing I am curious about from your numbers. How fast is lake self-check on the machine you were running these benchmarks on? That gives a good startup baseline for Lake to see how much room there is to improve.

@kim-em
Copy link
Collaborator

kim-em commented Aug 31, 2023

lake self-check is taking 0.08s.

@tydeu
Copy link
Member Author

tydeu commented Aug 31, 2023

@semorrison

lake self-check is taking 0.08s.

Wow! That is also a much greater difference between startup and configure times than the speed center benchmark. As such, I would curious in a more fine-grained profile of the Lake's performance in mathlib after merging all these performance improvements. That is, using something like Hotspot (like Sebastian's previous analysis).

@tydeu tydeu merged commit 4cc1ca7 into leanprover:master Aug 31, 2023
@tydeu tydeu deleted the lakefile-olean branch August 31, 2023 19:37
kim-em added a commit that referenced this pull request Sep 15, 2023
kim-em added a commit to kim-em/lean4 that referenced this pull request Sep 15, 2023
kim-em added a commit that referenced this pull request Sep 17, 2023
* chore: add release notes for #2470 and #2480

* chore: begin development cycle for 4.2.0

* chore: add Lake-related release notes for v4.1.0

---------

Co-authored-by: Mac Malone <[email protected]>
0x450x6c pushed a commit to 0x450x6c/lean4 that referenced this pull request Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Waiting for someone to review the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants