A whole bunch of optimizations in hot paths in the IO processing areas
of our code (well, one of them covers everything). I validated that each
commit either improved one or more of our vtebench results, or improved
the time it takes to process 2 years worth (2.4GB) of data from
asciinema.
## vtebench
<img width="1278" height="903" alt="image"
src="https://github.com/user-attachments/assets/bad46777-4606-4870-b7d7-8df0c4bb3b39"
/>
(I decided to patch vtebench to report in nanoseconds instead of
milliseconds since clearly it was not designed for a machine as fast as
mine. Nanoseconds gives much more useful results when the numbers are
this low.)
Do note the *slight* regression in the "unicode" test, this is probably
because I added a branch hint in `Terminal.print` in order to optimize
for printing narrow characters, since they make up the vast majority of
characters typically printed in the terminal, but the vtebench "unicode"
test is pretty much all wide characters.
This shouldn't have a negative effect on users of CJK languages since
it's a *very* slight reduction in speed and they will still be printing
many narrow characters, especially in TUIs; spaces, box drawing
characters, symbols, punctuation, etc.
## asciinema processing
I wrote a program that uses libghostty to push 2 years worth (2.4GB) of
data from publicly uploaded asciinema recordings in to the terminal as
fast as possible- since it's just libghostty, there's no renderer
overhead happening, it's just the core terminal emulation, effectively
everything that io-reader thread does if it didn't have wait for the
renderer ever.
On main, this took roughly 26.1–26.7 seconds to process, on this branch
it takes just 18.4–18.6 seconds, that's a ~30% improvement in raw IO
processing speed when processing real world data!
## Summary of changes
In order of commits:
- Fixed a bug that I hit when trying to have Ghostty process all that
asciinema data, in certain bad cases it was possible to accidentally
insert the `0` hyperlink ID in to a page, which would then cause a
lockup in ReleaseFast mode when trying to clone that page since the
string alloc would try to iterate `1..0` to allocate 0 chunks.
- I noticed in profiling Ghostty that `std.debug.assert` was showing up
in the profile, which it should not have been since its doc comment
promises that it will be optimized out in ReleaseFast- but evidently
something is wrong with Zig, or that comment's promise is based on an
expectation from LLVM that it fails to meet - but either way, by
replacing all uses of `assert` with a version that is explicitly marked
`inline`, that function call overhead in tight loops and hotpaths is
avoided. This change alone accounts for like a third of the IO
processing time improvement, though it had minimal impact on vtebench
scores.
- I optimized the SGR parser somewhat by adding branch hints and
removing the `.reset_underline` action, replacing it with `.{ .underline
= .none }`.
- Gated a somewhat expensive assert in RefCountedSet behind a runtime
safety check.
- Improved the performance of `Style.eql` and `Style.hash` since these
are hot functions, called extremely frequently since adding styles to
the style set is a very common operation. Achieved this by making `eql`
less generic - explicitly comparing each part of the style rather than
looping over fields - and ordering checks from most likely to differ to
least likely to differ so that differences can be found as soon as
possible; and changed the hash from xxhash to simply folding the packed
struct down to 64 bits and then using `std.hash.int`. Also manually
inlined the code from `std.meta.activeTag` in `Packed.fromStyle`, since
profiling showed it in the callstack and it's a single cast so it really
should not have the function call overhead.
- Explicitly marked some trivial functions as inline, the optimizer
would already have been doing this (probably) but doing it explicitly
gives the optimizer more time to spend on other things. Added cold
branch hints to "should be impossible" and error-returning paths that
should be very rare, and unlikely branch hints to a lot of "invalid"
paths- to optimize for receiving valid data.
- Removed a branch in the parser csi param action, just unconditionally
multiply by 10 before adding digit value, even if it's the first digit.
This codepath is rarely hit since we have a fast path for this in the
stream code, but the stream code already has this optimization so I just
copied it over.
- `CharsetState.charsets` used to be an `EnumArray`, but the
layout/access logic for that was less-than-ideal, and the access
functions were not inlining-- and these are very hot since we access
this for every single print, so I wrote a bespoke struct to hold that
info instead, gained a couple percent of IO perf with that.
- Added branch hints based on the data I derived from the asciinema
dump, which gave big boost to vtebench results, especially for the
cursor movement and dense cells tests (which makes sense, since cursor
movement and setting attributes both got `likely` hints :p) -- data at
https://github.com/qwerasd205/asciinema-stats
- This is probably the most invasive change in this PR: I removed the
dirty bitset from `Page` and replaced it with a dirty flag on each row,
for the majority of operations this is faster to write, since the row
being dirtied is probably already loaded and probably will be written to
for other changes as well. This gave a couple percent IO processing
improvement. The only exception is scrolling-type operations, which are
extremely efficient by just moving rows around with a single memmov, so
looping through the rows to mark each dirty slows them down, and indeed
after this change the scrolling benchmarks in vtebench regressed,
*however*...
- Added a "full page dirty" flag on `Page`, which is set when an
operation is performed that dirties most or all the rows in the page,
which is used for scrolling-type operations. This *does* make the dirty
tracking slightly less precise for these operations, but with the
caching and stuff we do in the renderer, I don't think `rebuildCells` is
a bottleneck, so rebuilding a few extra rows shouldn't hurt. After this
change, all the scrolling benchmarks in vtebench improved drastically.
- Tiny micro-improvements to RefCountedSet; streamlined the control flow
in `lookup`, added an unlikely branch hint in `insert` for the branch
that resurrects dead items since dead items aren't that common.
- Improve SGR parser performance again by using `@call(.always_inline`
to explicitly inline calls to `StaticBitSet.isSet` (for the separator
list), since I noticed they weren't being inlined, causing function call
overhead in a hotpath.
- I noticed that `clearGrapheme` and `clearHyperlink` would check every
cell in the row after they were done in order to update the
`grapheme`/`hyperlink` flag on the row if there were none left, which
isn't great since `clearCells` called these functions for multiple cells
in the same row back-to-back, which leads to a ton of excess work. I
separated the flag updating parts of these functions out and called them
only if necessary (if the cells being cleared were the full row then the
flag could unconditionally be set to false) and only after all the cells
were cleared. This gave a nice improvement to IO processing since
clearCells is evidently a very hot function.
- Removed inline annotations on `Page.clearGrapheme` and
`Page.clearHyperlink` in favor of inlining directly at the one callsite
that benefited from inlining, this improved IO processing speed.
- Inlined trivial function `Charset.table`.
- Inlined `size.getOffset` and `size.intFromBase` as they are both
trivial pointer math that often benefits from surrounding context.
---
If you'd like me to separate out the trivial improvements (branch hints,
inline annotations, 1-line changes) from the functionality-changing ones
(pretty much just the changes to dirty tracking), just let me know!
These were actually hurting performance lol, except in the places where
I added the `.always_inline` calls- for some reason if these functions
aren't inlined there it really messes up the top region scrolling
benchmark in vtebench and I'm not entirely certain why...
This improves the `clearCells` function since it only has to update once
after clearing all of the individual cells, or not at all if the whole
row was cleared since then it knows for sure that it cleared them all.
This also makes it so that the row style flag is properly tracked when
cells are cleared but not the whole row.
This adds a benchmark and some test coverage for a `screen-clone`
benchmark. This benchmarks the screen cloning which is a hot spot for
lock contention for the renderer + IO threads. I wasn't able to
meaningfully speed this up, but still want to commit this benchmark.
This could cause a 0-length hyperlink to be present in the screen,
which, in ReleaseFast, causes a lockup as the string alloc tries to
iterate `1..0` to allocate 0 chunks.
It was previously possible for `eraseRow` to move the cursor pin to a
different page, and then the call to `cursorChangePin` would try to free
the cursor style from that page even though that's not the page it
belongs to, which creates memory corruption in release modes and
integrity violations or assertions in debug mode.
As a bonus, this should actually be faster this way than the old code,
since it avoids needless work that `cursorChangePin` otherwise does.
These can be unambiguously invoked in certain parser states, and as such
we need to handle them. In real world use they are extremely rare, hence
the branch hint. Without this, we get illegal behavior by trying to cast
the value to the 7-bit C0 enum.
Chugging along towards #189
This adds significantly more internal work for searching. A long time
ago, I added #2885 which had a hint of what I was thinking of. This
simultaneously builds on this and changes direction.
The change of direction is that instead of making PageList fully
concurrency safe and having a search thread access it concurrently, I'm
now making an architectural shift where our search thread will grab the
big lock (blocking all IO/rendering), but with the bet that we can make
our critical areas small enough and time them well enough that the
performance hit while actively searching will be minimal. **Results yet
to be seen, but the path to implement this is much, much simpler.**
## Rearchitecting Search
To that end, this PR builds on #2885 by making `src/terminal/search` and
entire package (rather than a single file).
```mermaid
graph TB
subgraph Layer5 ["<b>Layer 5: Thread Orchestration</b>"]
Thread["<b>Thread</b><br/>━━━━━━━━━━━━━━━━━━━━━<br/>• MPSC queue management<br/>• libxev event loop<br/>• Message handling<br/>• Surface mailbox communication<br/>• Forward progress coordination"]
end
subgraph Layer4 ["<b>Layer 4: Screen Coordination</b>"]
ScreenSearch["<b>ScreenSearch</b><br/>━━━━━━━━━━━━━━━━━━━━━<br/>• State machine (tick + feed)<br/>• Result caching<br/>• Per-screen (alt/primary)<br/>• Composes Active + History search<br/>• Interrupt handling"]
end
subgraph Layer3 ["<b>Layer 3: Domain-Specific Search</b>"]
ActiveSearch["<b>ActiveSearch</b><br/>━━━━━━━━━━━━━━━━━━━━━<br/>• Active area only<br/>• Invalidate & re-search<br/>• Small, volatile data"]
PageListSearch["<b>PageListSearch</b><br/>━━━━━━━━━━━━━━━━━━━━━<br/>• History search (reverse order)<br/>• Separated tick/feed ops<br/>• Immutable PageList assumption<br/>• Garbage pin detection"]
end
subgraph Layer2 ["<b>Layer 1: Primitive Operations</b>"]
SlidingWindow["<b>SlidingWindow</b><br/>━━━━━━━━━━━━━━━━━━━━━<br/>• Manual linked list node management<br/>• Circular buffer maintenance<br/>• Zero-allocation search<br/>• Match yielding<br/>• Page boundary handling"]
end
Thread --> ScreenSearch
ScreenSearch --> ActiveSearch
ScreenSearch --> PageListSearch
ActiveSearch --> SlidingWindow
PageListSearch --> SlidingWindow
classDef layer5 fill:#0a0a0a,stroke:#ff0066,stroke-width:3px,color:#ffffff
classDef layer4 fill:#0f0f0f,stroke:#ff6600,stroke-width:3px,color:#ffffff
classDef layer3 fill:#141414,stroke:#ffaa00,stroke-width:3px,color:#ffffff
classDef layer2 fill:#1a1a1a,stroke:#00ff00,stroke-width:3px,color:#ffffff
class Thread layer5
class ScreenSearch layer4
class ActiveSearch,PageListSearch layer3
class SlidingWindow layer2
style Layer5 fill:#050505,stroke:#ff0066,stroke-width:2px,color:#ffffff
style Layer4 fill:#080808,stroke:#ff6600,stroke-width:2px,color:#ffffff
style Layer3 fill:#0c0c0c,stroke:#ffaa00,stroke-width:2px,color:#ffffff
style Layer2 fill:#101010,stroke:#00ff00,stroke-width:2px,color:#ffffff
```
Within the package, we have composable layers that let us test each
point:
- `SlidingWindow`: The lowest layer, the caller manually adds linked
list page nodes and it maintains a sliding window we search over,
yielding results without allocation (besides the circular buffers to
maintain the sliding window).
- `PageListSearch`: Searches a PageList structure in reverse order
(assumption: more recent matches are more valuable than older), but
separates out the `tick` (search, but no PageList access) and `feed`
(PageList access, prep data for search but don't search) operations.
This lets us `feed` in a critical area and `tick` outside. **This
assumes an immutable PageList, so this is for history.**
- `ActiveSearch`: Searches only the active area of a PageList. The
expectation is that the active area changes much more regularly, but it
is also very small (relative to scrollback). Throws away and re-searches
the active area as necessary.
- `ScreenSearch`: Composes the previous three components to coordinate
searching an active terminal screen. You'd have one of these per screen
(alt vs primary). This also caches results unlike the other components,
with the expectation that the caller will revisit the results as screens
change (so if you switch from neovim back to your shell and vice versa
with a search active, it won't start over).
- `Thread`: A dedicated search thread that will receive messages via
MPSC queues while managing the forward progress of a `ScreenSearch` and
sending matches back to the surface mailbox for apprt rendering. **The
thread component is not functional, just boilerplate, in this PR.**
ScreenSearch is a state machine that moves in an iterative `tick` +
`feed` fashion. This will let us "interrupt" the search with updates on
the search thread (read our mailbox via libxev loops for example) and
will let us minimize critical areas with locks (only `feed`).
Each component is significantly unit tested, especially around page
boundary cases. Given the complexity, there is no way this is perfect,
but the architecture is such that we can easily add regression tests as
we find issues.
## Other Changes, Notes
The only change to actually used code is that tracked pins in a
`PageList` can now be flagged as "garbage." A garbage tracked pin is one
that had to be moved in a non-sensical way because the previous location
it tracked has been deleted. This is used by the searcher to detect that
our history was pruned.
**If my assumption about the big lock is wrong** and this ends up being
godawful for performance, then it should still be okay because more
granular locking and reference counting such as that down by @dave-fl in
#8850 can be pushed into these components and reused. So this work is
still valuable on its own.
## Future
This PR is still just a bunch of internals, split out into its own PR so
I don't make one huge 10K diff PR. There are a number of future tasks:
- Flesh out `ScreenSearch` and hook it up to `Thread`
- Pull search thread management into `Surface` (or possibly the render
thread or shared render state since active area changes can be
synchronized with renderer frame rebuilds. Not sure yet.)
- Send updates back to the surface thread so that apprts can update UI.
- Apprt actions, input bindings, etc. to hook this all up (the easy
part, really).
The next step is to continue to flesh out the `ScreenSearch` as required
and hook it up to `Thread`.
**AI disclosure:** AI reviewed the code and assisted with some tests,
but didn't write any of the logic or design. This is beyond its ability
(or my ability to spec it out clearly enough for AI to succeed).
Fixes#9579
Protect against panics caused by integer overflows by using functions
that allow integer overflows to be caught instead of causing a panic.
Also protect against DOS from images that might not cause an
overflow but do consume an absurd amount of memory by limiting
images to a maximum size of 4GiB (which is the maximum size of
`image-storage-limit`).
