Skip to content

Performance optimizations for non-ASCII input - #4889

Merged
junegunn merged 8 commits into
masterfrom
devel
Aug 8, 2026
Merged

Performance optimizations for non-ASCII input#4889
junegunn merged 8 commits into
masterfrom
devel

Conversation

@junegunn

@junegunn junegunn commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Contribution Policy

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

All contributions are expected to demonstrate:

  • A clear understanding of the codebase
  • Alignment with product direction
  • Thoughtful reasoning behind changes
  • Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.


Acknowledgement

  • I confirm that this PR meets the above expectations and reflects my own understanding and real-world context.

- Capacity was byte length, over-allocating by bytes-per-rune (2-4x)
- Count non-continuation bytes with SWAR before allocating
- Invalid bytes undercount, never overcount, so append covers the gap
- Query performance unchanged, this is a memory fix
- The gain tracks bytes-per-rune, the cost tracks how much of the line
  follows the first non-ASCII byte, so the two move independently

Measured on 1.4M-line corpora:

- Every line CJK: RSS 362MB -> 255MB, ingestion -5%
- Mostly-ASCII paths behind a Hangul prefix: RSS 556MB -> 533MB,
  ingestion +3.5%, the counting pass covering the whole line
- The same paths with the Hangul at the end: RSS 563MB -> 535MB,
  ingestion +0.9%, the counting pass covering six bytes
utf8.DecodeRune already fast-paths ASCII, but it is too complex to inline
(cost 201 against a budget of 80), so a mostly-ASCII line pays one call
per byte just to be told the byte is ASCII.

Only the run after the first non-ASCII byte reaches the decode loop, so
the gain depends on where that byte falls.

- 70-rune ASCII line: 145ns -> 42ns in the decode loop
- Ingestion of 1.4M mostly-ASCII paths behind a Hangul prefix, where the
  loop covers the whole line: 377ms -> 233ms
- The same paths with the Hangul at the end, where it covers six bytes:
  207ms -> 196ms
- Break-even sits at ~100% non-ASCII runes: still 1.01x at 95%. Only a
  line holding no ASCII byte at all loses, by ~0.14ns per rune, ~5% of
  the loop
@junegunn
junegunn requested a balanced review from Copilot August 8, 2026 02:10
@junegunn junegunn self-assigned this Aug 8, 2026
@junegunn junegunn added the enhancement Enhancement to existing feature label Aug 8, 2026
@github-actions github-actions Bot added docs Documentation go Go code build Build system test Tests labels Aug 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Optimizes fuzzy matching and ingestion for non-ASCII input using rune-array prefiltering and cached folding metadata.

Changes:

  • Adds portable and architecture-optimized rune scanners.
  • Tracks foldable Unicode input and prefilters rune-mode matches.
  • Adds differential tests, fuzzing, and release notes.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/util/chars.go Adds rune counting and folding metadata.
src/util/chars_test.go Tests conversion, flags, and struct size.
src/algo/algo.go Enables rune-array prefiltering.
src/algo/runeprefilter_test.go Adds equivalence and fuzz tests.
src/algo/runeindex_x86.go Adds optimized little-endian rune scanners.
src/algo/runeindex_ref.go Adds portable reference scanners.
src/algo/runeindex_others.go Selects reference scanners on other architectures.
Makefile Adds the new fuzz target.
CHANGELOG.md Documents performance improvements.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/util/chars.go Outdated
Comment thread src/util/chars.go
Comment thread src/util/chars_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/algo/algo.go:388

  • This byte-scanner branch also accepts negative runes because it only checks the upper bound. Once isAscii correctly sends such patterns through the rune prefilter, this condition must select indexRune; otherwise byte(-1) searches for U+00FF and falsely rejects an exact rune(-1) match.
		if last < utf8.RuneSelf {

src/util/chars.go:56

  • This merged range also marks U+1F00–U+1FFF (Greek Extended), plus punctuation and symbols up to U+2184, as foldable even though they cannot become ASCII. Any item containing one of those runes therefore sets flagMayFold, and an ASCII query bypasses the new prefilter entirely. Split the Latin blocks and isolated compatibility characters so Greek Extended receives the optimization promised above.
	{0x1D00, 0x2184},

Comment thread src/algo/algo.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/util/chars.go:56

  • This merged interval also marks Greek Extended (U+1F00–U+1FFF) and unrelated blocks such as General Punctuation as foldable, even though those runes do not lowercase or normalize to ASCII. As a result, polytonic-Greek or punctuation-heavy lines set flagMayFold and bypass the new prefilter, contradicting the stated Greek exclusion; the script test only covers the basic Greek block. Split this interval into the actual Latin/subscript/stray ranges (or generate the exact bitmap) and cover Greek Extended in the test.
	{0x1D00, 0x2184},

asciiFuzzyIndex gave up on non-ASCII lines, so every item ran the full
score matrix. A []rune is a fixed 4-byte stride, so the SIMD byte
scanners can run over it directly: find the low byte, then confirm
4-byte alignment and three zero bytes.

Case folding and normalization can turn a non-ASCII rune into the ASCII
char being searched, which the scan cannot see. ToChars now flags lines
holding such a rune and those keep the old path. Normalization is
Latin-only, so Hangul, CJK, Cyrillic, Greek, Hebrew, Arabic, Thai, kana
and emoji never set the flag.

Chars had no spare padding, so inBytes moves into a flags byte.

Measured on 1.4M-line corpora:

- Mostly-ASCII paths behind a Hangul prefix: 'conf' 1.8x, 'binutils'
  2.9x, 'ltversion' 4.0x, no-match 8.4x
- Every line CJK: 17x on both matching and non-matching queries
- ASCII input unchanged, non-ASCII patterns not covered yet
normalizeRune guarded with 0x00C0..0xFF61, which does not exclude Hangul,
CJK or Cyrillic, so every rune of those scripts hashed into the map only
to miss. Every key of the map folds to ASCII, so the bitmap added for the
rune prefilter rejects them without a lookup.

- Non-ASCII queries 1.21x where every line is CJK, 1.05x on mostly-ASCII
  paths behind a Hangul prefix
- ASCII queries unchanged, the prefilter already skips Phase 2 for them
- Normalization share of query time for a non-ASCII query: 17.2% -> 0%
The scan only ran for ASCII patterns, so searching CJK text with a CJK
query still built the full score matrix. Scan for one byte of the pattern
rune and verify all four.

Which byte matters. Every ASCII rune contributes three zero bytes, so
U+AE00 scanned by its zero low byte hits on nearly every character of an
ASCII-heavy line. Pick a byte that cannot occur in an ASCII rune, else
any non-zero one.

A non-ASCII pattern rune is safe only when no other rune lowercases onto
it. Uncased is not sufficient: U+00DF has no simple uppercase, yet U+1E9E
lowercases to it, so the foldable set is excluded too.

Measured on 1.4M-line corpora, with a non-ASCII query:

- Every line CJK: 5.1x to 10.2x
- Mostly-ASCII paths behind a Hangul prefix: 5.6x to 6.0x, and 1.2x
  where every line matches so nothing can be rejected
- ASCII queries unchanged, kept off the non-inlinable guard
ToRunes aliases the rune array, and the editing actions append into
t.input in place when the cursor is not at the end, so the keystrokes
edit the item. Non-ASCII items only, ASCII gets a fresh slice.

    printf '한글abcde\n' | fzf --bind 'ctrl-y:replace-query'
    ctrl-y, Left, BSpace, ctrl-u  ->  한글abcee

Runes and ToRunes are now documented read-only. A stale fold bit was the
other symptom, letting the prefilter reject an item the general path
matches.
Lint/AmbiguousBlockAssociation. rubocop -a rewrote this on every
'make lint' run, leaving the tree dirty.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@junegunn
junegunn merged commit 715d26f into master Aug 8, 2026
9 checks passed
@junegunn
junegunn deleted the devel branch August 8, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build system docs Documentation enhancement Enhancement to existing feature go Go code test Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants