Skip to content

Add specialized SIMD line seeking routines #408

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

Merged
merged 5 commits into from
Jun 5, 2025

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jun 3, 2025

The previous memchr loop had the fatal flaw that it would break out
of the SIMD routines every time it hit a newline. This resulted in a
throughput drop down to ~250MB/s on my system in the worst case.
By writing SIMD routines specific to newline seeking, we can bump
that up by >500x. Navigating through a 1GB of text now takes ~16ms
independent of the contents.

@@ -1229,7 +1229,7 @@ impl TextBuffer {
}

let (delta, line) =
unicode::newlines_backward(chunk, chunk.len(), result.logical_pos.y, y);
simd::lines_bwd(chunk, chunk.len(), result.logical_pos.y, result.logical_pos.y);
Copy link
Member

Choose a reason for hiding this comment

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

this seems like an important change - arg 4?

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot to push before opening the PR. 😅

/// seeks backwards even if the `line` is already at `line_stop`.
/// This allows you to ensure (or test) whether `offset` is at a line start.
///
/// It returns an offset *past* the newline (= at the start of the next line).
Copy link
Member

Choose a reason for hiding this comment

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

next -> previous?

Suggested change
/// It returns an offset *past* the newline (= at the start of the next line).
/// It returns an offset *past* the newline (= at the start of the prior line).

or perhaps I misunderstand

Copy link
Member Author

Choose a reason for hiding this comment

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

It's more like "the current line". I changed the wording a bit to be clearer about this.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

Test on ARM64?

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

tested and benched on an rpi 3b and 5

@DHowett DHowett merged commit 065fa74 into main Jun 5, 2025
3 checks passed
@DHowett DHowett deleted the dev/lhecker/optimized-line-seeking branch June 5, 2025 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants