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

perf: fix linearity in (HashSet|HashMap).erase #3887

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Conversation

hargoniX
Copy link
Contributor

Fixes linearity issues in HashSet/HashMap erase functions.

IR before patch:

def Lean.HashMapImp.erase._rarg (x_1 : obj) (x_2 : obj) (x_3 : obj) (x_4 : obj) : obj :=
  let x_5 : obj := proj[0] x_3;
  inc x_5;
  let x_6 : obj := proj[1] x_3;
  inc x_6;
  let x_7 : obj := Array.size ◾ x_6;
  inc x_4;
  let x_8 : obj := app x_2 x_4;
  let x_9 : u64 := unbox x_8;
  dec x_8;
  let x_10 : usize := _private.Lean.Data.HashMap.0.Lean.HashMapImp.mkIdx x_7 x_9 ◾;
  let x_11 : obj := Array.uget ◾ x_6 x_10 ◾;
  inc x_11;
  inc x_4;
  inc x_1;
  let x_12 : u8 := Lean.AssocList.contains._rarg x_1 x_4 x_11;
  case x_12 : u8 of
  Bool.false →
    dec x_11;
    dec x_6;
    dec x_5;
    dec x_4;
    dec x_1;
    ret x_3
  Bool.true →
    let x_13 : u8 := isShared x_3;
    case x_13 : u8 of
    Bool.false →
      let x_14 : obj := proj[1] x_3;
      dec x_14;
      let x_15 : obj := proj[0] x_3;
      dec x_15;
      let x_16 : obj := 1;
      let x_17 : obj := Nat.sub x_5 x_16;
      dec x_5;
      let x_18 : obj := Lean.AssocList.erase._rarg x_1 x_4 x_11;
      let x_19 : obj := Array.uset ◾ x_6 x_10 x_18 ◾;
      set x_3[1] := x_19;
      set x_3[0] := x_17;
      ret x_3
    Bool.true →
      dec x_3;
      let x_20 : obj := 1;
      let x_21 : obj := Nat.sub x_5 x_20;
      dec x_5;
      let x_22 : obj := Lean.AssocList.erase._rarg x_1 x_4 x_11;
      let x_23 : obj := Array.uset ◾ x_6 x_10 x_22 ◾;
      let x_24 : obj := ctor_0[Lean.HashMapImp.mk] x_21 x_23;
      ret x_24

IR after the patch:

def Lean.HashMapImp.erase._rarg (x_1 : obj) (x_2 : obj) (x_3 : obj) (x_4 : obj) : obj :=
  let x_5 : u8 := isShared x_3;
  case x_5 : u8 of
  Bool.false →
    let x_6 : obj := proj[0] x_3;
    let x_7 : obj := proj[1] x_3;
    let x_8 : obj := Array.size ◾ x_7;
    inc x_4;
    let x_9 : obj := app x_2 x_4;
    let x_10 : u64 := unbox x_9;
    dec x_9;
    let x_11 : usize := _private.Lean.Data.HashMap.0.Lean.HashMapImp.mkIdx x_8 x_10 ◾;
    let x_12 : obj := Array.uget ◾ x_7 x_11 ◾;
    inc x_12;
    inc x_4;
    inc x_1;
    let x_13 : u8 := Lean.AssocList.contains._rarg x_1 x_4 x_12;
    case x_13 : u8 of
    Bool.false →
      dec x_12;
      dec x_4;
      dec x_1;
      ret x_3
    Bool.true →
      let x_14 : obj := 1;
      let x_15 : obj := Nat.sub x_6 x_14;
      dec x_6;
      let x_16 : obj := Lean.AssocList.erase._rarg x_1 x_4 x_12;
      let x_17 : obj := Array.uset ◾ x_7 x_11 x_16 ◾;
      set x_3[1] := x_17;
      set x_3[0] := x_15;
      ret x_3
  Bool.true →
    let x_18 : obj := proj[0] x_3;
    let x_19 : obj := proj[1] x_3;
    inc x_19;
    inc x_18;
    dec x_3;
    let x_20 : obj := Array.size ◾ x_19;
    inc x_4;
    let x_21 : obj := app x_2 x_4;
    let x_22 : u64 := unbox x_21;
    dec x_21;
    let x_23 : usize := _private.Lean.Data.HashMap.0.Lean.HashMapImp.mkIdx x_20 x_22 ◾;
    let x_24 : obj := Array.uget ◾ x_19 x_23 ◾;
    inc x_24;
    inc x_4;
    inc x_1;
    let x_25 : u8 := Lean.AssocList.contains._rarg x_1 x_4 x_24;
    case x_25 : u8 of
    Bool.false →
      dec x_24;
      dec x_4;
      dec x_1;
      let x_26 : obj := ctor_0[Lean.HashMapImp.mk] x_18 x_19;
      ret x_26
    Bool.true →
      let x_27 : obj := 1;
      let x_28 : obj := Nat.sub x_18 x_27;
      dec x_18;
      let x_29 : obj := Lean.AssocList.erase._rarg x_1 x_4 x_24;
      let x_30 : obj := Array.uset ◾ x_19 x_23 x_29 ◾;
      let x_31 : obj := ctor_0[Lean.HashMapImp.mk] x_28 x_30;
      ret x_31

Previously x_6 (the buckets array) always gets incremented, now only if the HashMap itself is shared.

@hargoniX
Copy link
Contributor Author

!bench

digama0 added a commit to leanprover-community/batteries that referenced this pull request Apr 12, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 12, 2024 08:34 Inactive
@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 321f8c0.
There were significant changes against commit 2e3d523:

  Benchmark   Metric          Change
  ===========================================
- stdlib      type checking     2.0% (24.0 σ)

@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 12, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Std CI can not be attempted yet, as the nightly-testing-2024-04-12 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Std CI should run now. (2024-04-12 08:38:20)

@hargoniX hargoniX added this pull request to the merge queue Apr 12, 2024
@hargoniX
Copy link
Contributor Author

These results are most likely just noise, there are no real callers of erase in our benched codepaths. The main impact will be in the LSP.

Merged via the queue into master with commit 723c340 Apr 12, 2024
11 checks passed
digama0 added a commit to leanprover-community/batteries that referenced this pull request Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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