feat: add Pane Search Phase 3 checkpoint (v1.3.3) - #46
Merged
Conversation
Performance and edge-case hardening for the pane search, keeping the existing Terminal search stack as the only engine. - Coalesce live-typing searches per pane (50 ms, leading + trailing); the callback re-reads the search box at fire time so the latest query always wins, navigation stays synchronous on current text, and fires after close are no-ops. - Converge an open search during sustained output: a non-debounced 500 ms refresh cap (new internal SearchRefreshNeeded event) complements the debounced OutputIdle path and is armed only while a search is active; search closed keeps the output path at one relaxed atomic load. - Repaint the scrollbar mark bitmap only when its content inputs change (geometry, categories, search generation, pip color, buffer mutation id while generic marks render); plain scrolling stops re-enumerating occurrences and mark rows. - Stop the resize/reflow invalidation from converting pre-reflow spans into a stray selection; the GH#19358 select-on-close behavior now belongs to the close path only. - Drop stored search highlights on main/alt screen-buffer switches; search keeps following the active buffer. - Release the terminal-side highlight copy when search clears. - Deterministic regression tests: mutation invalidation, focused-match anchoring, scrollback eviction, reflow + no-stray-selection, alt-screen transitions, generation/arming semantics, repaint-signature contracts, wide-character spans, and a log-only scan bench (WINTERM_SEARCH_BENCH_LINES scales it locally). - Version 1.3.3 across branding, packaging, module, scripts, READMEs, CHANGELOG, and progress docs; v1.3.3 appended to the checkpoint-tag allowlists. Engineering checkpoint only: no build label, no artifacts, Latest/WinGet/prerelease metadata untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pane Search Phase 3 — Performance & Edge-Case Hardening (v1.3.3)
Summary
Engineering checkpoint
1.3.3, Phase 3 of the Pane Search roadmap. No newuser-facing search feature and no visual redesign: this phase makes the
Phase 1/2 search reliable and responsive under real terminal workloads —
rapid typing, sustained output, large buffers and match counts, resize and
reflow, alternate-screen applications — and closes the edge cases found by
tracing the actual hot paths. The existing Microsoft Terminal search stack
(
SearchBoxControl→TermControl→ControlCore::Search→Search→TextBuffer::SearchText, renderer highlights, scrollbar mark surface)remains the only engine; no index, no mirror, no second search path.
Baseline / Phase 2 state
mainat97ecb7e85(v1.3.2, PR #45): complete pane search UX withcompact SearchBox,
current / totalcounter, responsive layout states,scrollbar search overview with per-row dedup and current-match emphasis,
ShowMarks-independent rendering, pane-isolated state.
Performance investigation (before any change)
Traced, in the live code:
TextBox.TextChanged→SearchChanged→TermControl::_SearchChanged→ControlCore::Searchruns on the UI thread holding the terminal write lock; a stale query
(
Search::IsStale, needle/flags/mutation-id compare) triggersSearch::Reset→TextBuffer::SearchText— a full ICU scan of everycommitted row, once per keystroke, including every backspace step.
SharedState::outputIdle(til::throttled_func, 100 ms, debounce +trailing) →
OutputIdleevent →TermControl::_refreshSearch()→reset-only
Search. Because the throttle is debounced, sustainedoutput at intervals < 100 ms postpones it forever: with
tail -f-stylestreams the counter froze and highlights/overview drifted misaligned
(buffer rows shift under stored spans) until output paused. Spec §11/§12
scenarios failed on the baseline.
Search::IsStalecompares the buffer'sGetLastMutationId(), which increments on every mutable row access —output, eviction (
IncrementCircularBuffer), clears — and every newTextBuffer(resize/reflow, alt-screen entry) starts in a distinctid-space. Staleness detection is complete; refresh delivery was the gap.
Search::Resetallocation. Each reset allocates a fresh resultvector via
SearchText; the previous vector is extracted for rendererinvalidation and freed.
Terminal::SetSearchHighlightshowevercopy-assigned into its member, so clearing search retained the old
capacity indefinitely (16 bytes × matches after Esc).
(8 ms)
_updateScrollBar→_throttledUpdateScrollbar, which fullyrepainted the mark bitmap on every tick while the surface renders —
including pure thumb moves during scrolling — re-enumerating the entire
occurrence list (
ForEachDistinctSearchRow) and every mark row(
GetMarkRows, O(buffer height)) to produce an identical bitmap. Thebitmap's content does not depend on the scroll position.
_refreshSizeUnderLock→UserResize(main bufferreplaced via
TextBuffer::Reflow) →ClearSearch()→ OutputIdlerecomputes ≤ 100 ms later. Correct — except
ClearSearchalso performsthe GH#19358 "select the focused result" conversion, feeding pre-reflow
spans through the new buffer's scroll offset: every resize with an
active search planted a stray selection at arbitrary coordinates, which
Search::Resetthen used as its current-match anchor.TextBuffer(search follows the active buffer — native semantics,preserved) but left
Terminal::_searchHighlightsholding the otherbuffer's spans; the renderer could paint them at wrong positions for up
to one refresh interval.
til::ICU::CreateRegexalready appliesuregex_setTimeLimit(4096)anduregex_setStackLimit(4 MB): invalidpatterns fail fast into the existing invalid-regex status with results
cleared; pathological patterns abort bounded (silently truncating that
scan's results) rather than hanging the UI. Verified, documented, no
engine change.
_refreshSearchre-checksIsOpen(); revokers +weak refs guard destruction;
ClearSearchresets core state. Alreadysound; preserved under the new coalescing by re-reading live state.
Measurement: the in-tree
TestSearchScanPerfSmokebench (log-only) writesrepresentative log lines and times
ControlCore::Searchfore,er,error,ERROR, a no-match literal,ERROR|WARN, and an invalid regex.Release x64 on the development machine:
eerrorERROR|WARN(regex)[A full scan of the complete default-size buffer costs ≈ 2 ms and scales
linearly, so single keystrokes were never the visible problem — redundant
scans during bursts, the starved mid-output refresh, and the per-tick
scrollbar re-enumeration were. That shaped the changes: cheap bounded
coalescing (not a rework), a bounded refresh cap, and a repaint signature.
Timings are logged, never asserted;
WINTERM_SEARCH_BENCH_LINESscales thebench locally (32 k-row extrapolation ≈ 7–8 ms per scan).
Changes (all measurement-justified; nothing else touched)
TermControl): aThrottledFunc<>(50 ms, leading + trailing) now backs_SearchChanged. The leading edge keeps a single keystroke asresponsive as before; bursts collapse into one trailing search. The
callback captures no query: it re-reads the search box at fire time, so
the latest query always wins, a fire after Esc/close finds
IsOpen() == falseand does nothing, and TermControl teardown is guardedby the weak reference. Emptying the query bypasses the throttle and
clears synchronously (spec §26). Navigation (
Enter/Shift+Enter/buttons) keeps its direct synchronous path reading the box's current
text — it cannot act on a stale query by construction (spec §9).
ControlCore): a companiontil::throttled_func(500 ms, trailing, no debounce) raises the newinternal
SearchRefreshNeededevent →_refreshSearch(). It is armedfrom the output handler only while
_searchActive(atomic; set bynon-empty
Search, cleared by empty query andClearSearch). Sustainedoutput now updates counter/highlights/overview at most every 500 ms;
quiet terminals keep the untouched 100 ms OutputIdle path. Search closed:
the output path pays one relaxed load, schedules nothing (spec §5/§27).
TermControl+SearchUxHelpers):ScrollbarMarkPaintStatecaptures everything the bitmap depends on —geometry (maximum, viewport, pixel size), category flags, the new
ControlCore::SearchStateGeneration()(bumped on reset/navigate/clear),the pip color while search pips render, and
BufferMutationId()whilegeneric marks render. Ticks whose state equals the last painted state
skip the repaint; collapsing the canvas invalidates the signature. Pure
scrolling with tens of thousands of matches no longer re-enumerates
anything. Search core remains the source of truth; the signature is an
identity, not a cache of results.
ControlCore):ClearSearch()splitinto the public close path (still performs the GH#19358 focused-result
selection) and
_clearSearchImpl(false)for the resize/reflow path,which now only invalidates. Regression-tested.
Terminal):UseAlternateScreenBuffer/UseMainScreenBufferclear stored search highlights (they describe theother buffer); the next refresh recomputes against the active buffer.
Search semantics unchanged and now documented: search targets the
currently active buffer (alt screen while a TUI runs). No process-name
special cases anywhere.
Terminal):SetSearchHighlightstakes thevector by value and move-assigns, so clearing releases the old
allocation instead of retaining capacity (spec §28).
Explicitly not done: no second engine, no index/mirror/database, no regex
replacement, no UI redesign, no per-frame work, no polling threads, no
Sleep loops (spec §8/§29/§35).
Continuous-output behavior
17 matches → new ERROR → 18now converges ≤ 500 ms during sustainedoutput and ≤ 100 ms after idle; current match stays anchored to the focused
span (
Search::Resetre-anchors via the previous focused highlight), soappended matches update
4/20 → 4/25instead of resetting to1/25(deterministically tested). Bursts converge after the existing idle
boundary; per-chunk work is unchanged (one throttle poke).
Resize/reflow behavior
Resize invalidates (hides) results immediately, recomputes after the
existing idle boundary against the reflowed buffer, never renders old spans
against the new geometry, and no longer creates a selection. Covered by a
shrink-then-grow reflow test asserting cleared results, no stray selection,
and post-reflow span validity in the narrowed geometry.
Alternate-screen behavior
Buffer switches drop the other buffer's highlight spans immediately;
search follows the active buffer (
vim/lesssearch the alt screen,returning to the shell searches the main buffer again). Deterministically
tested through
?1049h/?1049ltransitions, including span bounds insidethe alt viewport.
Unicode/regex behavior
Span widths verified through ControlCore for Traditional/Simplified
Chinese, Japanese, Korean (wide cells), accented Latin (narrow), and an
emoji surrogate pair; navigation across wide matches stays in range.
Case-insensitive default and case-sensitive mode regression-covered by the
existing Phase 1 tests plus
ut_hostSearchTests (unchanged). Invalidregex keeps reporting the existing invalid status with zero retained
results; ICU's compiled-in time/stack limits bound pathological patterns
(risk documented above; no semantic change).
Tests
New in
UnitTests_Control/ControlCoreTests.cpp(all deterministic, nowall-clock assertions):
TestSearchBufferMutationRefreshesResults— mutation invalidation,post-output count refresh, focused-match anchoring, no-op refresh.
TestSearchScrollbackEvictionSafety— tiny history, evicted contentreports zero (not stale), surviving spans/current match in bounds.
TestSearchReflowInvalidationAndNoStraySelection— wrapped lines,shrink/grow reflow, cleared results, no selection, valid new spans.
TestSearchAltBufferTransitions— highlight clearing on both switches,search follows the active buffer, alt spans within the viewport.
TestSearchStateGenerationSemantics— generation bump/no-op rules,BufferMutationIdmovement,_searchActivearm/disarm lifecycle.TestSearchUnicodeWideSpans— span widths for 錯誤/错误/エラー/오류/café/👍.TestScrollbarMarkPaintStateContracts— repaint-signature rules,including mutation-id and pip-color participation gating.
TestSearchScanPerfSmoke— log-only scan-cost bench + invalid-regexstate assertions (
WINTERM_SEARCH_BENCH_LINESscales it locally).Manual performance checklist (for the 1.4.0-alpha validation pass)
Automated evidence in this PR: the TAEF suites above (mutation, eviction,
reflow, alt-screen, Unicode, generation/arming, repaint signature) and the
scan-cost bench at 2 000 and 9 001 lines. The interactive scenarios below
complement them and are the manual checklist for the upcoming alpha
validation; they were not run against a packaged build in this PR.
1..10000 | % { Write-Host "INFO request=$_ ERROR sample WARN payload" },then type
e→er→err→error; typing must stay fluid.THIS_STRING_DOES_NOT_EXIST_123456; no freeze.INFO; counter shows the bounded999+form, terminal and overview stay usable.while ($true) { Write-Host "$(Get-Date -Format o) INFO request ERROR sample"; Start-Sleep -Milliseconds 50 }(stop with
Ctrl+C), searchERROR; counter/highlights/overview mustconverge at least every ~500 ms while the loop runs.
after output idles.
border narrower and wider; no ghost or missing highlights, no stray
selection.
no cross-pane interference.
vim/lesswith a search open across alt-screenenter/exit; no stale highlights,
Escstill owned by the search box.錯誤/错误/エラー/오류/café/👍; counts, highlights, navigation correct.Escimmediately, repeatedly;nothing may resurrect afterwards.
Version changes
1.3.2→1.3.3across: Brandingversion.json+ReleaseMetadata.h,appx manifest,
WindowsTerminal.rc/wt.rc/winterm-shim.rc(dotted +comma forms), PowerShell module (
psd1/psm1/sharedversion.json),Workspace descriptor/serializer fallbacks, pinned literals in
package-shell-assets.ps1/test.ps1/verify-branding.ps1/test-visual-progress.ps1/verify-version.ps1, README.md + README.ja.mdsource-version references, CHANGELOG, current-progress.
v1.3.3appendedto the checkpoint-tag allowlists in
release.yml,test-release-workflow.ps1, andverify-version.ps1(existing tagsretained). No build label, no artifacts, Latest/WinGet/prerelease metadata
untouched.
Deferred final integration
1.4.0-alpha(final integration + manual user validation) starts only on aseparate instruction. Remaining known risks deferred there: ICU's bounded
pathological-regex truncation is documented rather than surfaced in UI, and
mid-output refresh cost on maximum (32k-line) histories is bounded but
measurable (≤ 2 full scans/s while streaming with search open).