Skip to content

Conversation

@nomeata
Copy link
Collaborator

@nomeata nomeata commented Apr 21, 2025

This PR introduces a noConfusionType construction that’s sub-quadratic in size, and reduces faster.

The previous noConfusion construction with two nested match statements is quadratic in size and reduction behavior. Using some helper definitions, a linear size construction is possible.

With this, processing the RISC-V-AST definition from https://github.com/opencompl/sail-riscv-lean takes 6s instead of 60s.

The previous construction is still used when processing the early prelude, and can be enabled elsewhere using set_option backwards.linearNoConfusionType false.

@nomeata
Copy link
Collaborator Author

nomeata commented Apr 21, 2025

!bench

@leanprover-bot
Copy link
Collaborator

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

@nomeata
Copy link
Collaborator Author

nomeata commented Apr 21, 2025

!bench

@tobiasgrosser
Copy link
Contributor

Thank you, @nomeata. This is very promising.

@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 Apr 21, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Apr 21, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 2386a3d7c7086c92266fe04257f5217a22f6993e --onto de27872f3ffee8f2eb13f36fbb8fec9bcdfe3b8c. You can force Mathlib CI using the force-mathlib-ci label. (2025-04-21 16:40:03)
  • 💥 Mathlib branch lean-pr-testing-8037 build failed against this PR. (2025-05-16 10:46:52) View Log
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 7994e55d8067de8ec4b52c105df7c71341e9c190 --onto ca9b3eb75f3f794d3d526a7c37da389175b317a2. You can force Mathlib CI using the force-mathlib-ci label. (2025-05-16 14:17:25)
  • ✅ Mathlib branch lean-pr-testing-8037 has successfully built against this PR. (2025-05-16 16:37:06) View Log
  • ✅ Mathlib branch lean-pr-testing-8037 has successfully built against this PR. (2025-05-16 19:47:07) View Log
  • ✅ Mathlib branch lean-pr-testing-8037 has successfully built against this PR. (2025-05-17 09:41:47) View Log
  • 🟡 Mathlib branch lean-pr-testing-8037 build against this PR was cancelled. (2025-05-17 16:17:25) View Log
  • ✅ Mathlib branch lean-pr-testing-8037 has successfully built against this PR. (2025-05-17 17:00:59) View Log
  • ❗ Batteries CI can not be attempted yet, as the nightly-testing-2025-05-22 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. You can force Mathlib CI using the force-mathlib-ci label. (2025-05-22 11:56:45)
  • ✅ Mathlib branch lean-pr-testing-8037 has successfully built against this PR. (2025-05-22 15:50:47) View Log

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 00155d0.
There were significant changes against commit 2386a3d:

  Benchmark                  Metric           Change
  ==============================================================
+ bv_decide_large_aig.lean   branch-misses     -1.6%   (-33.3 σ)
+ bv_decide_large_aig.lean   wall-clock        -4.7%   (-20.1 σ)
- channel.lean               bounded0_spsc     14.3%    (35.3 σ)
- channel.lean               unbounded_mpsc     6.6%    (12.4 σ)
- channel.lean               unbounded_spsc    24.4%    (14.1 σ)
+ ilean roundtrip            compress          -4.3%   (-10.5 σ)
+ lake build clean           task-clock        -2.3%   (-33.3 σ)
+ riscv-ast.lean             branch-misses    -52.9%  (-169.5 σ)
+ riscv-ast.lean             branches         -73.4% (-2827.9 σ)
+ riscv-ast.lean             instructions     -74.7% (-2855.8 σ)
+ riscv-ast.lean             maxrss           -46.3%   (-52.6 σ)
+ riscv-ast.lean             task-clock       -73.7%   (-76.2 σ)
+ riscv-ast.lean             wall-clock       -72.3%   (-70.7 σ)
+ stdlib                     blocked           -2.7%   (-11.0 σ)
+ stdlib                     maxrss            -1.4%   (-29.4 σ)
+ stdlib                     task-clock        -2.3%   (-18.4 σ)
+ stdlib                     wall-clock        -2.2%   (-66.8 σ)
+ unionfind                  task-clock        -6.8%   (-26.4 σ)
+ unionfind                  wall-clock        -6.8%   (-26.2 σ)

@nomeata
Copy link
Collaborator Author

nomeata commented Apr 21, 2025

!bench

@nomeata
Copy link
Collaborator Author

nomeata commented Apr 21, 2025

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit dfc1df4.
There were significant changes against commit 2386a3d:

  Benchmark        Metric          Change
  ===================================================
- liasolver        maxrss            1.1%    (24.1 σ)
- nat_repr         maxrss            1.1%    (24.1 σ)
- parser           maxrss            1.1%    (24.1 σ)
- qsort            maxrss            1.1%    (26.6 σ)
+ rbmap            task-clock       -1.2%   (-27.2 σ)
+ rbmap            wall-clock       -1.2%   (-15.7 σ)
+ riscv-ast.lean   branch-misses   -55.1%   (-54.7 σ)
+ riscv-ast.lean   branches        -73.5% (-1016.6 σ)
+ riscv-ast.lean   instructions    -74.8% (-1021.0 σ)
+ riscv-ast.lean   maxrss          -45.9%   (-41.8 σ)
+ riscv-ast.lean   task-clock      -75.5%   (-99.6 σ)
+ riscv-ast.lean   wall-clock      -73.8%  (-163.9 σ)
+ stdlib           maxrss           -1.4%   (-59.9 σ)

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 51cc087.
There were significant changes against commit 2386a3d:

  Benchmark                            Metric                  Change
  ===============================================================================
+ Init.Data.BitVec.Lemmas re-elab      branch-misses            -1.8%   (-14.9 σ)
+ Init.Data.BitVec.Lemmas re-elab      task-clock               -2.4%   (-12.9 σ)
+ Std.Data.Internal.List.Associative   branch-misses            -2.1%   (-10.4 σ)
+ big_omega.lean MT                    task-clock               -4.2%   (-17.5 σ)
+ big_omega.lean MT                    wall-clock               -4.5%   (-17.1 σ)
+ bv_decide_large_aig.lean             branch-misses            -1.4%   (-13.8 σ)
+ const_fold                           task-clock               -2.8%   (-74.9 σ)
+ const_fold                           wall-clock               -2.8%   (-67.9 σ)
+ lake build clean                     task-clock               -1.9%   (-75.1 σ)
+ omega_stress.lean async              branch-misses            -1.1%   (-10.3 σ)
+ parser                               task-clock               -8.4%   (-12.2 σ)
+ parser                               wall-clock               -8.4%   (-12.1 σ)
+ rbmap                                task-clock               -5.1%   (-14.7 σ)
+ rbmap                                wall-clock               -5.1%   (-14.1 σ)
+ rbmap_library                        task-clock               -5.2%   (-13.2 σ)
+ rbmap_library                        wall-clock               -5.2%   (-13.2 σ)
+ riscv-ast.lean                       branch-misses           -57.6%   (-98.7 σ)
+ riscv-ast.lean                       branches                -78.2% (-1320.3 σ)
+ riscv-ast.lean                       instructions            -79.4% (-1297.4 σ)
+ riscv-ast.lean                       maxrss                  -53.0%   (-21.6 σ)
+ riscv-ast.lean                       task-clock              -75.9%  (-284.4 σ)
+ riscv-ast.lean                       wall-clock              -75.1%  (-118.2 σ)
+ stdlib                               attribute application    -1.3%   (-15.2 σ)
+ stdlib                               blocked (unaccounted)    -1.9%   (-10.7 σ)
+ stdlib                               tactic execution         -1.6%   (-33.6 σ)
+ stdlib                               wall-clock               -1.9%   (-20.0 σ)

@Rob23oba
Copy link
Contributor

Rob23oba commented May 7, 2025

This seems very promising! I wonder if a similar strategy could be used for a linear DecidableEq construction... (In the sense of if h : x.toCtorIdx = y.toCtorIdx then match y with | ... => some kind of withCtor h else .isFalse (ne_of_apply_ne h))

@nomeata
Copy link
Collaborator Author

nomeata commented May 7, 2025

Yes, there is hope that we can apply this technique to the other currently quadratic constructions.

@tobiasgrosser
Copy link
Contributor

@nomeata, we are currently trying to get the lean definitions of sail-riscv being built by default and our CI times are a bit long. We already moved them from 36 minutes to 23 minutes, but now lean's compile and build time is on the critical path. Shaving off 1 minute due to this patch would be really nice. Hence, I wanted to express a 👍 to get this merged when you are ready.

@nomeata
Copy link
Collaborator Author

nomeata commented May 8, 2025

The roadmap has it not this quarter; this is mostly an experiment. I'd still have to find out if my construction works in general, whether it can fully replace the current one or whether it should kick in only for large inductives and clean it up.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 16, 2025
@nomeata nomeata reopened this May 17, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 17, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 17, 2025
@nomeata
Copy link
Collaborator Author

nomeata commented May 17, 2025

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit a1cecfd.
There were significant changes against commit ca9b3eb:

  Benchmark                 Metric                 Change
  =====================================================================
+ const_fold                task-clock              -2.5%     (-21.0 σ)
+ const_fold                wall-clock              -2.5%     (-20.5 σ)
+ nat_repr                  branches                -1.9%   (-1254.0 σ)
+ omega_stress.lean async   branches                -1.9%    (-118.4 σ)
+ omega_stress.lean async   instructions            -1.9%    (-116.4 σ)
+ riscv-ast.lean            branch-misses          -56.3%    (-700.2 σ)
+ riscv-ast.lean            branches               -77.6%  (-81839.3 σ)
+ riscv-ast.lean            instructions           -78.9% (-248079.5 σ)
+ riscv-ast.lean            maxrss                 -51.8%     (-20.2 σ)
+ riscv-ast.lean            task-clock             -76.0%     (-60.2 σ)
+ riscv-ast.lean            wall-clock             -75.1%     (-60.2 σ)
+ stdlib                    instantiate metavars    -1.6%     (-20.9 σ)

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 17, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 17, 2025
@nomeata
Copy link
Collaborator Author

nomeata commented May 17, 2025

Ok, mathlib benchmarks are inconspicious, as far as I can tell: https://speed.lean-lang.org/mathlib4/compare/58b8accd-a1ae-4b0e-9f85-674a4f295f9f/to/2e918b87-fd9c-462b-b3b9-a2b38b581368

So it’s plausible to use this constructions by default, not just for inductives with many constructors (to keep things uniform). I fear we still need the previous constructions for Init.Prelude before we have enough of Nat in place.

@nomeata
Copy link
Collaborator Author

nomeata commented May 22, 2025

Anyways, what I’d like to see is a fully general withFoo combinator, possibly in the variant you provided or with a default fallback, that eliminates into Sort u rather than Type u. But it seems that’s blocked by #7096.

I thought with #7096 fixed we can do that, but it seems that at least the code that I tried to use here would require even further improvements to the kernel level normalization around max and imax, e.g. to handle

  Sort (imax (u + 1) (u + 1) u_1)
but is expected to have type
  Sort (imax (u + 1) u_1)

@nomeata nomeata added the changelog-language Language features and metaprograms label May 22, 2025
@nomeata nomeata marked this pull request as ready for review May 22, 2025 14:12
@nomeata nomeata changed the title feat: linear noConfusionType construction feat: linear-size noConfusionType construction May 22, 2025
@nomeata nomeata enabled auto-merge May 22, 2025 14:14
@nomeata nomeata added this pull request to the merge queue May 22, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 22, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 22, 2025
Merged via the queue into master with commit 5e40f4a May 22, 2025
16 checks passed
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-language Language features and metaprograms merge-ci Enable merge queue CI checks for PR. In particular, produce artifacts for all major platforms. 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.

6 participants