improve StringView::find and StringView::rev_find

[MINGtoMING]

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]

Magic number 4 for algorithm selection threshold needs justification

Category
Performance
Code Snippet
if str.length() <= 4 { brute_force_find(self, str) } else { boyer_moore_horspool_find(self, str) }
Recommendation
Consider making the threshold configurable or add documentation explaining the choice of 4
Reasoning
The threshold value of 4 seems arbitrary. Different thresholds might be optimal for different platforms or usage patterns. Documentation would help future maintainers understand the decision.

Potential integer overflow in skip table indexing

Category
Correctness
Code Snippet
skip_table[needle.unsafe_charcode_at(i) & 0xFF] = needle_len - 1 - i
Recommendation
Add bounds checking or documentation explaining why overflow is not possible
Reasoning
The expression needle_len - 1 - i could potentially overflow for very large strings. While unlikely in practice, it should be documented or protected against.

TODO comment about Two-Way algorithm should be tracked

Category
Maintainability
Code Snippet
// TODO: When the pattern string is long (>= 256), consider using Two-Way algorithm
Recommendation
Convert TODO into a tracked issue or implement the suggested optimization
Reasoning
TODOs in code can be easily forgotten. For a performance-critical feature like string search, important optimization opportunities should be properly tracked.

[coveralls]

Pull Request Test Coverage Report for Build 49

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.03%) to 89.215%

---

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

Totals
Change from base Build 48:	0.03%
Covered Lines:	3491
Relevant Lines:	3913

---

💛 - Coveralls