stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 671 forks source link

chore: Remove unnecessary function `slice_partialeq()` #5148

Closed jbencin closed 2 months ago

jbencin commented 2 months ago

Description

I noticed this function while working on #5137. It's unnecessary, as == works fine with slices, and I'm sure Rust's PartialEq implementation uses something like memcmp() instead of a loop, so should improve performance. Using my standard benchmark, I see a small improvement in block processing times:

❯ hyperfine -w 3 -r 20 "target/release/stacks-inspect.develop replay-block /home/jbencin/data/next/mainnet range 99990 100000" "target/release/stacks-inspect.remove-slice_partialeq replay-block /home/jbencin/data/next/mainnet range 99990 100000"
Benchmark 1: target/release/stacks-inspect.develop replay-block /home/jbencin/data/next/mainnet range 99990 100000
  Time (mean ± σ):      9.753 s ±  0.044 s    [User: 9.042 s, System: 0.674 s]
  Range (min … max):    9.698 s …  9.886 s    20 runs

Benchmark 2: target/release/stacks-inspect.remove-slice_partialeq replay-block /home/jbencin/data/next/mainnet range 99990 100000
  Time (mean ± σ):      9.627 s ±  0.074 s    [User: 8.927 s, System: 0.661 s]
  Range (min … max):    9.540 s …  9.780 s    20 runs

Summary
  target/release/stacks-inspect.remove-slice_partialeq replay-block /home/jbencin/data/next/mainnet range 99990 100000 ran
    1.01 ± 0.01 times faster than target/release/stacks-inspect.develop replay-block /home/jbencin/data/next/mainnet range 99990 100000
jcnelson commented 2 months ago

We had this code because at one point, PartialEq didn't work for slices longer than 32 items.

blockstack-devops commented 2 weeks ago

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.