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

refactor: lake: use StateRefT for BuildStore #6290

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

tydeu
Copy link
Member

@tydeu tydeu commented Dec 2, 2024

This PR uses StateRefT instead of StateT to equip the Lake build monad with a build store.

As a IO reference, different threads may now contend with the build store. However, benchmark results indicate that this does not have a significant performance impact. On a synchronization front, the lack of a mutex should not be a concern because the build store is a memorization data structure and thus order is theoretically irrelevant.

@tydeu tydeu added the changelog-lake Lake label Dec 2, 2024
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Dec 2, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Dec 2, 2024

Mathlib CI status (docs):

  • ❗ Batteries CI can not be attempted yet, as the nightly-testing-2024-11-30 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Batteries CI should run now. (2024-12-02 19:14:25)
  • ✅ Mathlib branch lean-pr-testing-6290 has successfully built against this PR. (2025-01-15 01:52:47) View Log

@tydeu
Copy link
Member Author

tydeu commented Dec 2, 2024

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 79d8c56.
There were no significant changes against commit 27df5e9.

@tydeu tydeu force-pushed the lake/ref-build-store branch from 79d8c56 to 66d2c0b Compare January 15, 2025 00:41
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jan 15, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jan 15, 2025
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Jan 15, 2025
@tydeu
Copy link
Member Author

tydeu commented Jan 15, 2025

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
The entire run failed.
Found no significant differences.

@tydeu
Copy link
Member Author

tydeu commented Jan 15, 2025

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
The entire run failed.
Found no significant differences.

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
The entire run failed.
Found no significant differences.

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
There were no significant changes against commit e9bd980.

@tydeu
Copy link
Member Author

tydeu commented Jan 15, 2025

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
The entire run failed.
Found no significant differences.

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
The entire run failed.
Found no significant differences.

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 66d2c0b.
There were significant changes against commit e9bd980:

  Benchmark   Metric                 Change
  ===================================================
+ big_do      task-clock              -3.0% (-12.9 σ)
+ big_do      wall-clock              -3.0% (-12.6 σ)
+ liasolver   task-clock              -2.8% (-21.2 σ)
+ liasolver   wall-clock              -2.9% (-12.9 σ)
+ rbmap_1     task-clock              -4.3% (-13.4 σ)
+ rbmap_1     wall-clock              -4.3% (-13.5 σ)
- stdlib      instantiate metavars     4.8%  (30.4 σ)

@tydeu tydeu marked this pull request as ready for review January 15, 2025 23:42
@tydeu tydeu added this pull request to the merge queue Jan 15, 2025
Merged via the queue into leanprover:master with commit 0050e93 Jan 16, 2025
19 checks passed
luisacicolini pushed a commit to opencompl/lean4 that referenced this pull request Jan 21, 2025
This PR uses `StateRefT` instead of `StateT` to equip the Lake build
monad with a build store.

As a IO reference, different threads may now contend with the build
store. However, benchmark results indicate that this does not have a
significant performance impact. On a synchronization front, the lack of
a mutex should not be a concern because the build store is a
memorization data structure and thus order is theoretically irrelevant.
JovanGerb pushed a commit to JovanGerb/lean4 that referenced this pull request Jan 21, 2025
This PR uses `StateRefT` instead of `StateT` to equip the Lake build
monad with a build store.

As a IO reference, different threads may now contend with the build
store. However, benchmark results indicate that this does not have a
significant performance impact. On a synchronization front, the lack of
a mutex should not be a concern because the build store is a
memorization data structure and thus order is theoretically irrelevant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR changelog-lake Lake toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants