Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions library/core/src/str/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,13 @@ impl TwoWaySearcher {
let start =
if long_period { self.crit_pos } else { cmp::max(self.crit_pos, self.memory) };
for i in start..needle.len() {
if needle[i] != haystack[self.position + i] {
// SAFETY: on every iteration of `'search`, the `haystack.get(self.position + needle_last)`
// check returned `Some`, so `self.position + needle_last < haystack.len()`.
// Since `i < needle.len()` implies `i <= needle_last`, we have
// `self.position + i < haystack.len()`.
// Every path that mutates `self.position` below either returns or re-enters `'search`,
// which re-runs the check before reaching the loop again.
if needle[i] != unsafe { *haystack.get_unchecked(self.position + i) } {

@Mark-Simulacrum Mark-Simulacrum Jun 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale here makes sense to me. I do wonder if we could tell LLVM enough that it would see it -- e.g. maybe subslicing needle and then having haystack[self.position..][..needle_slice.len()] would be enough? But I'm OK with this one as-is too.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to do...

let haystack_window = &haystack[self.position..][..needle.len()];

... to then use it in the condition (I believe that's what you meant)...

if needle[i] != haystack_window[i] {

... but the benchmarks results ended up being way worse than the implementation currently on the PR (on the newly added benchmarks):

Benchmark Latest version of the PR LLVM trick Delta
find_str 8,190 ns 16,167 ns +97%
rfind_str 11,800 ns 10,539 ns −11%
find_str_worst_case 1,104 ns 2,070 ns +87%
rfind_str_worst_case 36,630 ns 38,000 ns +4%

self.position += i - self.crit_pos + 1;
if !long_period {
self.memory = 0;
Expand All @@ -1506,7 +1512,13 @@ impl TwoWaySearcher {
// See if the left part of the needle matches
let start = if long_period { 0 } else { self.memory };
for i in (start..self.crit_pos).rev() {
if needle[i] != haystack[self.position + i] {
// SAFETY: on every iteration of `'search`, the `haystack.get(self.position + needle_last)`
// check returned `Some`, so `self.position + needle_last < haystack.len()`.
// Since `i < self.crit_pos <= needle.len()`, we have `i <= needle_last`, and thus
// `self.position + i <= self.position + needle_last < haystack.len()`.
// Every path that mutates `self.position` below either returns or re-enters `'search`,
// which re-runs the check before reaching the loop again.
if needle[i] != unsafe { *haystack.get_unchecked(self.position + i) } {
self.position += self.period;
if !long_period {
self.memory = needle.len() - self.period;
Expand Down Expand Up @@ -1581,7 +1593,14 @@ impl TwoWaySearcher {
cmp::min(self.crit_pos_back, self.memory_back)
};
for i in (0..crit).rev() {
if needle[i] != haystack[self.end - needle.len() + i] {
// SAFETY: On every iteration of `'search`, `haystack.get(self.end.wrapping_sub(needle.len()))`
// returned `Some`, so `self.end >= needle.len()` and `self.end - needle.len() < haystack.len()`.
// Since `self.end <= haystack.len()` and `i < needle.len()`, we have
// `self.end - needle.len() + i < self.end <= haystack.len()`, so
// `haystack.get_unchecked(self.end - needle.len() + i)` is safe.
// - The path that mutates `self.end` either re-enters `'search`, which re-runs the checks
// before reaching this loop again, or returns on match, so the invariant holds.
if needle[i] != unsafe { *haystack.get_unchecked(self.end - needle.len() + i) } {
self.end -= self.crit_pos_back - i;
if !long_period {
self.memory_back = needle.len();
Expand All @@ -1593,7 +1612,12 @@ impl TwoWaySearcher {
// See if the right part of the needle matches
let needle_end = if long_period { needle.len() } else { self.memory_back };
for i in self.crit_pos_back..needle_end {
if needle[i] != haystack[self.end - needle.len() + i] {
// SAFETY: The same `self.end - needle.len() + i < haystack.len()` argument as the
// left-part loop applies: the `haystack.get(self.end.wrapping_sub(needle.len()))`
// check at the top of `'search` established the bound for this iteration, and
// every mutation of `self.end` is followed by `continue 'search` (which re-runs
// the check) or a `return` (which exits before any further unsafe access).
if needle[i] != unsafe { *haystack.get_unchecked(self.end - needle.len() + i) } {
self.end -= self.period;
if !long_period {
self.memory_back = self.period;
Expand Down
48 changes: 48 additions & 0 deletions library/coretests/benches/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,51 @@ fn ends_with_str(b: &mut Bencher) {
}
})
}

fn make_haystack() -> String {
"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse quis lorem \
sit amet dolor ultricies condimentum. Praesent iaculis purus elit, ac malesuada \
quam malesuada in. Duis sed orci eros. Suspendisse sit amet magna mollis, mollis \
nunc luctus, imperdiet mi. Integer fringilla non sem ut lacinia. Fusce varius \
tortor a risus porttitor hendrerit. Morbi mauris dui, ultricies nec tempus vel, \
gravida nec quam. In est dui, tincidunt sed tempus interdum, adipiscing laoreet \
ante. Etiam tempor, tellus quis sagittis interdum, nulla purus mattis sem, quis \
auctor erat odio ac tellus. In nec nunc sit amet diam volutpat molestie at sed \
ipsum. Vestibulum laoreet consequat vulputate. Integer accumsan lorem ac dignissim \
placerat. Suspendisse convallis faucibus lorem. Aliquam erat volutpat."
.repeat(50)
}

#[bench]
fn find_str(b: &mut Bencher) {
let s = make_haystack();
let haystack = black_box(s.as_str());
b.bytes = haystack.len() as u64;
b.iter(|| black_box(haystack.find("the english language")))
}

#[bench]
fn rfind_str(b: &mut Bencher) {
let s = make_haystack();
let haystack = black_box(s.as_str());
b.bytes = haystack.len() as u64;
b.iter(|| black_box(haystack.rfind("the english language")))
}

#[bench]
fn find_str_worst_case(b: &mut Bencher) {
let near_miss = "the english languagX";
let haystack_str = near_miss.repeat(2000);
let haystack = black_box(haystack_str.as_str());
b.bytes = haystack.len() as u64;
b.iter(|| black_box(haystack.find("the english language")))
}

#[bench]
fn rfind_str_worst_case(b: &mut Bencher) {
let near_miss = "the english languagX";
let haystack_str = near_miss.repeat(2000);
let haystack = black_box(haystack_str.as_str());
b.bytes = haystack.len() as u64;
b.iter(|| black_box(haystack.rfind("the english language")))
}
1 change: 1 addition & 0 deletions typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ extend-exclude = [
# generated lorem ipsum texts
"library/alloctests/benches/str.rs",
"library/alloctests/tests/str.rs",
"library/coretests/benches/pattern.rs",
]

[default.extend-words]
Expand Down
Loading