Skip to content

Avoid LINQ allocations in OcrFixEngine.HasMostlyUppercaseLetters - #13383

Merged
niksedk merged 2 commits into
SubtitleEdit:mainfrom
ivandrofly:perf/ocr-has-mostly-uppercase-letters
Aug 8, 2026
Merged

Avoid LINQ allocations in OcrFixEngine.HasMostlyUppercaseLetters#13383
niksedk merged 2 commits into
SubtitleEdit:mainfrom
ivandrofly:perf/ocr-has-mostly-uppercase-letters

Conversation

@ivandrofly

Copy link
Copy Markdown
Member

Summary

  • Rewrite HasMostlyUppercaseLetters to count letters and uppercase letters in a single pass over the string instead of allocating an intermediate char[] via Where(...).ToArray() and iterating it twice
  • Replace the uppercaseCount > letterCount / 2.0 double comparison with equivalent integer math (uppercaseCount * 2 > letterCount)
  • Behavior is unchanged; the method is called from GetSpellCheckSuggestions for every misspelled OCR word

Benchmark

BenchmarkDotNet 0.15.8, .NET 10.0.10, Windows 11, i7-13700, ShortRun job:

Word LINQ (before) Single-pass (after) Speedup Allocated before → after
"" 0.38 ns ~0 ns 0 → 0
123-456 22.1 ns 1.8 ns 12× 56 B → 0
hello 31.7 ns 2.8 ns 11× 96 B → 0
Hello 33.3 ns 3.7 ns 96 B → 0
HELLO 32.3 ns 2.8 ns 12× 96 B → 0
L'HOMME 37.9 ns 4.2 ns 96 B → 0
EXTRAORDINARY 74.5 ns 7.4 ns 10× 112 B → 0
extraordinarily 80.6 ns 8.5 ns 112 B → 0

Test plan

  • dotnet build SubtitleEdit.sln succeeds
  • Run OCR spell check on a subtitle containing a misspelled mostly-uppercase word (e.g. HELL0) and verify the suggestion list still offers an uppercase variant first
  • Verify lowercase misspelled words and non-letter tokens get the same suggestions as before (no uppercase variant inserted)

🤖 Generated with Claude Code

Count letters and uppercase letters in a single pass instead of
allocating an intermediate array, and compare counts with integer
math instead of dividing by 2.0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

This PR optimizes OcrFixEngine.HasMostlyUppercaseLetters to avoid LINQ allocations and reduce per-word overhead during OCR spell-check suggestion generation.

Changes:

  • Replaced Where(...).ToArray() + Count(...) with a single-pass character scan.
  • Switched the “mostly uppercase” threshold comparison from floating-point math to integer math.

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

Comment thread src/libuilogic/Ocr/FixEngine/OcrFixEngine.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@niksedk
niksedk merged commit 9308bc9 into SubtitleEdit:main Aug 8, 2026
1 check passed
}

return uppercaseCount * 2 > letterCount;
return uppercaseCount > letterCount / 2;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is not fixing anything, if not making it slower, - this was my first solution but then find it was slower!

Image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is * 2 confusing or?

niksedk added a commit to ivandrofly/subtitleedit that referenced this pull request Aug 9, 2026
Round 4 of the micro-perf hunt, in the same vein as SubtitleEdit#13383: the waste is
allocations and rescans in helpers that run once per subtitle line, per
word or per character.

Change casing (the big one)
- StrippableText's name loop lower-cased every entry of the name list on
  every paragraph. For English that list is ~8000 names, so a 200-line
  subtitle allocated 1.6 million throwaway strings. "lower" is already
  lower case, so an OrdinalIgnoreCase search finds the same positions -
  which is what the loop's own continuation search already used.
- Hoisted the name-end character set (it concatenated a literal with
  Environment.NewLine per candidate match) and replaced two
  sb.ToString().EndsWith(..) suffix tests, which copied the whole
  accumulated line per character, with an in-place compare.
- FixCasingAfterTitles took a Substring of the rest of the line for every
  character position; it now compares the tail in place.
- FixCasing ran RemoveHtmlTags twice on the same text per paragraph.

Character counting
- CalcCjk.IsCjk allocated a one-character string and ran a regex over it
  for every character outside two hard-coded ranges - and the grid re-reads
  CPS and line length on every repaint. Now tests the block ranges directly;
  CalcCjkTest pins it to the old regex for all 65536 chars.
- CalcNoSpaceCpsOnly / CalcNoSpaceOrPunctuationCpsOnly allocated a new
  calculator per call, throwing away CalcFactory's memoization.

Subtitle formats
- SubStationAlpha: ported the two fixes [V4+ Styles] already had - style
  lookup through a HashSet instead of a linear list scan per paragraph,
  and TrimBuilder instead of copying the finished output twice.
- MicroDVD: ten tag branches each counted a tag over the whole line before
  the cheap StartsWith that rejects it; operands swapped.
- SAMI: dropped an uppercased copy of each cue that fed an already
  ignore-case search, replaced two substring+uppercase character scans,
  hoisted a character set out of a per-character loop, and built the
  milliseconds string in the StringBuilder that was already in scope.
- SubViewer 2.0's IsMine joined the whole file into one string to look for
  "[br]", which cannot straddle a line.
- Regex.Match(x).Success -> IsMatch(x) in 32 places (allocates a Match plus
  its group machinery per line, for a bool).

Hearing impaired / fix common errors / OCR / spell check
- Utilities.IsAllUppercase and HasUppercase replace "s == s.ToUpperInvariant()"
  and "s != s.ToLowerInvariant()" at 11 sites; both are pinned to the string
  comparison they replace for every character.
- The uppercase whitelist set was rebuilt from settings on every line.
- ReInsertHtmlTags did two dictionary probes per character; TryGetValue now.
- Helper.FixDash ran RemoveHtmlTags twice per call and counted at most three
  lines through LINQ with a TrimStart string per line.
- OcrFixReplaceList2: four ContainsKey+indexer pairs each building their key
  twice, two inline char[] allocations and a path scan with a concatenation,
  all per OCR'd word.
- SpellCheckWordLists built both candidate phrases inside the loop over the
  user phrase list rather than once.

Verified with BenchmarkDotNet (Apple M4, .NET 10), same benchmarks run
against a stashed baseline:

| Benchmark              | Before    | After     | Ratio | Alloc before | Alloc after |
|------------------------|-----------|-----------|-------|--------------|-------------|
| FixCasingNormal (200)  | 53.05 ms  | 26.26 ms  | 0.50  | 63.16 MB     | 1.64 MB     |
| LoadSami (500)         | 1.554 ms  | 1.211 ms  | 0.78  | 4.47 MB      | 2.04 MB     |
| SubStationAlphaToText  | 628.8 us  | 473.2 us  | 0.75  | 879.7 KB     | 617.6 KB    |
| MicroDvdToText (500)   | 189.5 us  | 151.4 us  | 0.80  | 338.0 KB     | 338.0 KB    |
| CalcCjk CountLatin     | 3.172 us  | 2.184 us  | 0.69  | 2496 B       | 1248 B      |
| RemoveHearingImpaired  | 1.159 ms  | 1.128 ms  | 0.97  | 2.62 MB      | 2.61 MB     |
| AutoBreak (tagged)     | 10.375 us | 10.251 us | 0.99  | 11.72 KB     | 11.72 KB    |

The last two are within noise - those changes are allocation hygiene, not
a measurable win, and are kept because they are strictly less work.

Behaviour: 948 libse + 149 libuilogic tests pass, and a round-trip harness
over 14 formats (write, read back, IsMine; plain and styled input) produces
byte-identical output before and after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ivandrofly
ivandrofly deleted the perf/ocr-has-mostly-uppercase-letters branch August 9, 2026 16:35
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.

3 participants