improve StringView::find and StringView::rev_find

[lin-zhenming]

Bench code

///|
test (it : @bench.T) {
  let str = String::from_array(@quickcheck.samples(100000))
  let needle = str.substring(start=50000, end=50002)
  for _ in 0..<10 {
    assert_eq(find_v1(str, needle), find_v2(str, needle))
    assert_eq(rev_find_v1(str, needle), rev_find_v2(str, needle))
  }
  it.bench(name="find_v1/short_needle", fn() { it.keep(find_v1(str, needle)) })
  it.bench(name="find_v2/short_needle", fn() { it.keep(find_v2(str, needle)) })
  it.bench(name="rev_find_v1/short_needle", fn() {
    it.keep(rev_find_v1(str, needle))
  })
  it.bench(name="rev_find_v2/short_needle", fn() {
    it.keep(rev_find_v2(str, needle))
  })
  let needle = str.substring(start=50021, end=50191)
  for _ in 0..<10 {
    assert_eq(find_v1(str, needle), find_v2(str, needle))
    assert_eq(rev_find_v1(str, needle), rev_find_v2(str, needle))
  }
  it.bench(name="find_v1/long_needle", fn() { it.keep(find_v1(str, needle)) })
  it.bench(name="find_v2/long_needle", fn() { it.keep(find_v2(str, needle)) })
  it.bench(name="rev_find_v1/long_needle", fn() {
    it.keep(rev_find_v1(str, needle))
  })
  it.bench(name="rev_find_v2/long_needle", fn() {
    it.keep(rev_find_v2(str, needle))
  })
}

///|
fn find_v1(haystack : @string.View, needle : @string.View) -> Int? {
  haystack.find(needle)
}

///|
fn find_v2(haystack : @string.View, needle : @string.View) -> Int? {
  if needle.length() <= 4 {
    brute_force_find(haystack, needle)
  } else {
    boyer_moore_horspool_find(haystack, needle)
  }
}

///|
fn brute_force_find(haystack : @string.View, needle : @string.View) -> Int? {
  let haystack_len = haystack.length()
  let needle_len = needle.length()
  guard needle_len > 0 else { return Some(0) }
  guard haystack_len >= needle_len else { return None }
  let needle_first = needle.unsafe_charcode_at(0)
  let forward_len = haystack_len - needle_len
  let mut i = 0
  while i <= forward_len {
    while i <= forward_len && haystack.unsafe_charcode_at(i) != needle_first {
      i += 1
    }
    if i <= forward_len {
      for j in 1..<needle_len {
        if haystack.unsafe_charcode_at(i + j) != needle.unsafe_charcode_at(j) {
          break
        }
      } else {
        return Some(i)
      }
      i += 1
    }
  }
  None
}

///|
fn boyer_moore_horspool_find(
  haystack : @string.View,
  needle : @string.View
) -> Int? {
  let haystack_len = haystack.length()
  let needle_len = needle.length()
  guard needle_len > 0 else { return Some(0) }
  guard haystack_len >= needle_len else { return None }
  let skip_table = FixedArray::make(1 << 8, needle_len)
  for i in 0..<(needle_len - 1) {
    skip_table[needle.unsafe_charcode_at(i) & 0xFF] = needle_len - 1 - i
  }
  for i = 0
      i <= haystack_len - needle_len
      i = i + skip_table[haystack.unsafe_charcode_at(i + needle_len - 1) & 0xFF] {
    for j in 0..=(needle_len - 1) {
      if haystack.unsafe_charcode_at(i + j) != needle.unsafe_charcode_at(j) {
        break
      }
    } else {
      return Some(i)
    }
  }
  None
}

///|
fn rev_find_v1(haystack : @string.View, needle : @string.View) -> Int? {
  haystack.rev_find(needle)
}

///|
fn rev_find_v2(haystack : @string.View, needle : @string.View) -> Int? {
  if needle.length() <= 4 {
    brute_force_rev_find(haystack, needle)
  } else {
    boyer_moore_horspool_rev_find(haystack, needle)
  }
}

///|
fn brute_force_rev_find(haystack : @string.View, needle : @string.View) -> Int? {
  let haystack_len = haystack.length()
  let needle_len = needle.length()
  guard needle_len > 0 else { return Some(0) }
  guard haystack_len >= needle_len else { return None }
  let needle_first = needle.unsafe_charcode_at(0)
  let mut i = haystack_len - needle_len
  while i >= 0 {
    while i >= 0 && haystack.unsafe_charcode_at(i) != needle_first {
      i -= 1
    }
    if i >= 0 {
      for j in 1..<needle_len {
        if haystack.unsafe_charcode_at(i + j) != needle.unsafe_charcode_at(j) {
          break
        }
      } else {
        return Some(i)
      }
      i -= 1
    }
  }
  None
}

///|
fn boyer_moore_horspool_rev_find(
  haystack : @string.View,
  needle : @string.View
) -> Int? {
  let haystack_len = haystack.length()
  let needle_len = needle.length()
  guard needle_len > 0 else { return Some(0) }
  guard haystack_len >= needle_len else { return None }
  let skip_table = FixedArray::make(1 << 8, needle_len)
  for i = needle_len - 1; i > 0; i = i - 1 {
    skip_table[needle.unsafe_charcode_at(i) & 0xFF] = i
  }
  for i = haystack_len - needle_len
      i >= 0
      i = i - skip_table[haystack.unsafe_charcode_at(i) & 0xFF] {
    for j in 0..<needle_len {
      if haystack.unsafe_charcode_at(i + j) != needle.unsafe_charcode_at(j) {
        break
      }
    } else {
      return Some(i)
    }
  }
  None
}

Bench result

• native

name                     time (mean ± σ)         range (min … max) 
find_v1/short_needle        2.69 µs ±   0.06 µs     2.60 µs …   2.77 µs  in 10 ×  39006 runs
find_v2/short_needle        2.39 µs ±   0.01 µs     2.38 µs …   2.41 µs  in 10 ×  42477 runs
rev_find_v1/short_needle    4.53 µs ±   0.17 µs     4.31 µs …   4.79 µs  in 10 ×  22830 runs
rev_find_v2/short_needle    4.43 µs ±   0.04 µs     4.38 µs …   4.50 µs  in 10 ×  22856 runs
find_v1/long_needle        23.13 µs ±   1.61 µs    21.71 µs …  26.52 µs  in 10 ×   4543 runs
find_v2/long_needle         3.94 µs ±   0.38 µs     3.50 µs …   4.60 µs  in 10 ×  25307 runs
rev_find_v1/long_needle    20.50 µs ±   0.23 µs    20.14 µs …  20.75 µs  in 10 ×   4836 runs
rev_find_v2/long_needle     3.05 µs ±   0.02 µs     3.03 µs …   3.09 µs  in 10 ×  32979 runs

• wasm-gc

name                     time (mean ± σ)         range (min … max) 
find_v1/short_needle        6.46 µs ±   0.46 µs     5.84 µs …   7.56 µs  in 10 ×  16262 runs
find_v2/short_needle        5.62 µs ±   0.06 µs     5.56 µs …   5.74 µs  in 10 ×  17996 runs
rev_find_v1/short_needle   12.41 µs ±   1.88 µs     9.72 µs …  14.28 µs  in 10 ×  10055 runs
rev_find_v2/short_needle   10.19 µs ±   0.35 µs     9.87 µs …  11.05 µs  in 10 ×   8767 runs
find_v1/long_needle        54.45 µs ±   6.80 µs    46.40 µs …  63.30 µs  in 10 ×   2161 runs
find_v2/long_needle         4.45 µs ±   0.48 µs     3.78 µs …   4.96 µs  in 10 ×  18510 runs
rev_find_v1/long_needle    55.77 µs ±   5.62 µs    47.37 µs …  64.36 µs  in 10 ×   2211 runs
rev_find_v2/long_needle     4.24 µs ±   0.49 µs     3.47 µs …   5.06 µs  in 10 ×  20872 runs

• wasm

name                     time (mean ± σ)         range (min … max) 
find_v1/short_needle        7.39 µs ±   0.68 µs     6.46 µs …   8.84 µs  in 10 ×  15974 runs
find_v2/short_needle        6.40 µs ±   0.51 µs     5.96 µs …   7.41 µs  in 10 ×  13584 runs
rev_find_v1/short_needle   11.20 µs ±   0.43 µs    10.58 µs …  11.81 µs  in 10 ×   9365 runs
rev_find_v2/short_needle   11.79 µs ±   0.32 µs    11.34 µs …  12.34 µs  in 10 ×   8568 runs
find_v1/long_needle        57.55 µs ±  10.78 µs    50.54 µs …  78.73 µs  in 10 ×   1928 runs
find_v2/long_needle         4.26 µs ±   0.04 µs     4.22 µs …   4.33 µs  in 10 ×  22875 runs
rev_find_v1/long_needle    52.96 µs ±   4.45 µs    48.13 µs …  61.40 µs  in 10 ×   1974 runs
rev_find_v2/long_needle     4.16 µs ±   0.03 µs     4.10 µs …   4.20 µs  in 10 ×  23717 runs

• js

name                     time (mean ± σ)         range (min … max) 
find_v1/short_needle        5.62 µs ±   0.34 µs     5.33 µs …   6.15 µs  in 10 ×  14740 runs
find_v2/short_needle        5.40 µs ±   0.04 µs     5.35 µs …   5.48 µs  in 10 ×  18347 runs
rev_find_v1/short_needle    9.69 µs ±   0.15 µs     9.56 µs …   9.99 µs  in 10 ×  10410 runs
rev_find_v2/short_needle    9.64 µs ±   0.06 µs     9.56 µs …   9.73 µs  in 10 ×  10256 runs
find_v1/long_needle        52.12 µs ±  15.45 µs    45.05 µs …  93.30 µs  in 10 ×    831 runs
find_v2/long_needle         9.19 µs ±   0.11 µs     9.00 µs …   9.32 µs  in 10 ×  11014 runs
rev_find_v1/long_needle    52.67 µs ±  16.22 µs    45.49 µs …  95.28 µs  in 10 ×    820 runs
rev_find_v2/long_needle     8.45 µs ±   0.21 µs     8.16 µs …   8.84 µs  in 10 ×  11783 runs

[peter-jerry-ye-code-review]

Inconsistent empty needle handling between find and rev_find functions

Category
Correctness
Code Snippet
Lines 33-34 (brute_force_find): guard needle_len > 0 else { return Some(0) }
Lines 168-169 (brute_force_rev_find): guard needle_len > 0 else { return Some(haystack_len) }
Recommendation
Both functions should return the same value for empty needles. Consider standardizing on Some(0) for consistency, or document why they differ.
Reasoning
The find function returns Some(0) for empty needle while rev_find returns Some(haystack_len). This inconsistency could lead to confusion and bugs when using both functions interchangeably.

Skip table creation uses bitwise masking that may cause hash collisions

Category
Performance
Code Snippet
Lines 68-70 (boyer_moore_horspool_find): skip_table[needle.unsafe_charcode_at(i) & 0xFF] = needle_len - 1 - i
Lines 191-193 (boyer_moore_horspool_rev_find): skip_table[needle.unsafe_charcode_at(i) & 0xFF] = i
Recommendation
Consider using a larger skip table (e.g., 1 << 16) or a different hashing strategy for Unicode characters to reduce collisions and improve performance for non-ASCII text.
Reasoning
The current implementation only uses the lower 8 bits of character codes, which can cause many Unicode characters to map to the same skip table entry, potentially degrading performance for international text.

Code duplication between forward and reverse Boyer-Moore-Horspool implementations

Category
Maintainability
Code Snippet
Lines 60-80 (boyer_moore_horspool_find) and Lines 183-203 (boyer_moore_horspool_rev_find) have very similar structure with only minor differences in skip table building and iteration direction
Recommendation
Extract common logic into a generic helper function that takes direction as a parameter, or create a shared skip table building function to reduce duplication.
Reasoning
The two Boyer-Moore-Horspool implementations share most of their logic. Reducing duplication would make the code easier to maintain and reduce the chance of bugs when making changes to the algorithm.

[coveralls]

Pull Request Test Coverage Report for Build 408

Details

• 46 of 48 (95.83%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 90.135%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
string/methods.mbt	46	48	95.83%

Totals
Change from base Build 400:	0.02%
Covered Lines:	3545
Relevant Lines:	3933

---

💛 - Coveralls

[cursor]

Bug: Reverse Search Skip Table Logic Error

The boyer_moore_horspool_rev_find function's skip table is incorrectly constructed and used. It applies forward Boyer-Moore-Horspool skip table logic (storing i as the skip value) and uses the first character of the current window (haystack.unsafe_charcode_at(i)) for lookup, which is unsuitable for a reverse search. This leads to incorrect and inefficient skipping behavior.

string/methods.mbt#L200-L207

core/string/methods.mbt

Lines 200 to 207 in 041feb6

	guard haystack_len >= needle_len else { return None }
	let skip_table = FixedArray::make(1 << 8, needle_len)
	for i = needle_len - 1; i > 0; i = i - 1 {
	skip_table[needle.unsafe_charcode_at(i) & 0xFF] = i
	}
	for i = haystack_len - needle_len
	i >= 0
	i = i - skip_table[haystack.unsafe_charcode_at(i) & 0xFF] {

Fix in Cursor • Fix in Web

---

BugBot free trial expires on July 22, 2025
Learn more in the Cursor dashboard.

Was this report helpful? Give feedback by reacting with 👍 or 👎