mirror of
https://github.com/Mudlet/Mudlet
synced 2026-08-13 18:26:27 -04:00
5 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Jay Howard
|
707a51b0d4
|
improve: the ISO 8859-1 decoder no longer allocates a string for every character (#9856)
#### Brief overview of PR changes/additions - The `ISO 8859-1` branch of `translateToPlainTextInner()` wrapped each decoded character in a temporary `QString` before appending it, costing one heap allocation per character on the per-character decode path. - Append the bare `QChar` instead, which is what the adjacent branch five lines above (bytes below 128 for table-based encodings) already does. - Second commit adds a `latin1_*` phase to `PipelineBenchmark`, which previously set no server encoding and so never entered this branch at all. #### Motivation for adding to Mudlet Removes a heap allocation per character of received text for every profile using ISO 8859-1 — worth about **23% more throughput** on that decode path. #### Other info (issues closed, discussion etc) **Measured** with the new `latin1_*` phase on macOS/arm64, Release, `-DUSE_SANITIZER=""`, two binaries from one toolchain run in 12 interleaved ABBA pairs so run-order effects cancel: | metric | before | after | delta | pairs favouring | t | |---|---|---|---|---|---| | `latin1_lines_per_sec` | 79,629 | 98,322 | **+23.47%** | 12/12 | +28.61 | | `latin1_best_pass_ms` | 314.0 | 254.3 | **-19.01%** | 12/12 | -25.58 | | `text_best_pass_ms` (control) | 294.9 | 296.2 | +0.42% | 4/12 | +0.13 | ISO 8859-1 has no lookup table, so **every** byte takes the changed line — roughly 2.4 million heap allocations removed per pass over the 2.4 MB corpus. The control metric not moving is the check that the delta is real. Note the first commit's message says "No benchmark figure is quoted"; that statement is superseded by the second commit and the table above. Caution when reading the phases: do not compare `latin1_*` against `text_*` directly. Latin-1 decoding is intrinsically cheaper than UTF-8, so that phase reads faster regardless of this change. Only the before/after of the same metric is meaningful. This is an inconsistency rather than a deliberate choice: both branches were written in the same commit (#969), five lines apart, and the ISO 8859-1 one was never revisited. `git log -L` over these lines shows only incidental touches since — CP437 support in #3579, the GBK/GB18030 decoder, a comment typo fix in #1495, a signed/unsigned cast pass, and the brace-formatting pass in #1115. Behaviour is unchanged by construction: both forms evaluate the same `QChar::fromLatin1(ch)` and hand the identical `QChar` to `QString::append()`; only the temporary `QString` disappears. **Test case:** `lua setServerEncoding("ISO 8859-1")` then `lua feedTriggers("caf\233 na\239ve \253\254 \160\176\191\208\247\n", false)` renders `café naïve ýþ °¿Ð÷`, exercising bytes 0xE9, 0xEF, 0xFD, 0xFE, 0xA0, 0xB0, 0xBF, 0xD0 and 0xF7 through the changed line. Output is byte-identical before and after, verified against before/after builds on macOS. Note for anyone testing: do not use byte 0xFF. `CHAR_END_OF_FILE '\xff'` (`TStringUtils.h:32`) is Mudlet's internal line-commit and prompt marker — `CHAR_IS_COMMIT_CHAR` lists it beside `\n` and `\r`, and `TBuffer.cpp:1668`/`:1684` use it to set `promptBuffer` — so it can never arrive as displayable text regardless of this change. `GlyphOverflowTest`, `CopyAsImageTest`, `WindowBackgroundTest`, `ProfileRoundTripTest`, `TriggerSameLineMatchTest`, `cTelnetBufferTest`, `TelnetSgrDefaultColorTest` and `TelnetStringSequenceRecoveryTest` all pass. --------- Signed-off-by: Jay Howard <jay.patrick.howard@gmail.com> |
||
|
Jay Howard
|
ea4fa62e82
|
fix: exact-match triggers no longer copy every line they check (#9853)
#### Brief overview of PR changes/additions - `match_exact_match()` did `QString text = haystack;` then chopped a trailing newline. The assignment is copy-on-write and cheap, but `chop()` mutates, forcing the detach: a heap allocation plus a full character copy of the line. - The newline is always present - `TMainConsole::runTriggers()` appends one to every line before dispatch (`TMainConsole.cpp:1570`) - so the chop always fires and the copy always happens, once per exact-match pattern, per reachable trigger, per line of game text. - Use a `QStringView`. Chopping a view moves only its own end pointer, so nothing is allocated, and the comparison against the needle is unchanged. Both chop one UTF-16 code unit, so behaviour is identical for every input. #### Motivation for adding to Mudlet Removes a per-line heap allocation and line copy from the trigger matching path, which runs for every line of game text. #### Other info (issues closed, discussion etc) Measured with `PipelineBenchmark` on macOS/arm64, Release, `-DUSE_SANITIZER=""`, two binaries from one toolchain run in 24 interleaved ABBA pairs so run-order effects cancel: trigger throughput **+3.28%** (t=6.41), trigger overhead **-6.37%** (t=-4.94). The text-only control moved +0.23% (t=-0.56, 12/24 paired wins), i.e. no effect, which is the check that the trigger deltas are real. Worth stating plainly: the stock benchmark corpus contains **no** exact-match patterns, so `match_exact_match()` is never entered by it. Twelve were added locally purely to measure this. The gain therefore scales with how many exact-match patterns a profile actually has, and is zero for a profile with none. **Test case:** behaviour-neutral, so the checks are for regressions around where the chop lands. Exact-match triggers fire correctly on a plain ASCII line, on a line with an accented character, on one with an em dash, and on one containing an emoji (a surrogate pair, i.e. two UTF-16 code units - the case most likely to expose a code-unit-based chop). A line with trailing whitespace before the newline correctly does *not* match. Verified against before/after builds on macOS, plus 5 trigger functional test suites. --------- Signed-off-by: Jay Howard <jay.patrick.howard@gmail.com> |
||
|
Vadim Peretokin
|
9a9710b229
|
fix: new profiles process game text about twice as fast (#9705)
#### Brief overview of PR changes/additions - The starter UI armed **77 always-active PCRE triggers** (12 chat + 65 vitals) at package load, so every line a game sent was matched against all of them - and every line one matched was then re-walked in Lua with all 77 patterns **recompiled from source**, because `rex.match` given a pattern string compiles it afresh on every call. They are now fronted by 4 triggers (3 chat-routing groups + 1 vitals prefilter) and compiled once. The 65 vitals shapes and 12 chat shapes are byte-identical and still do all the reading. - The plain-text vitals layer now retires itself once GMCP or MSDP holds the source lock, since `applyVitals` discards its readings from that point anyway, and re-arms on disconnect. - `PipelineBenchmark` created its profile through the production new-profile path, so the starter UI was **inside** the `text_lines_per_sec` baseline backing the "no more than 10% throughput loss" gate for #9011 - the guard built to catch this class of regression could not see it. Pipeline metrics now come from a profile with default packages suppressed; the shipped configuration is reported separately as `defaults_*` and gated in its own right. #### Motivation for adding to Mudlet Every new 5.0 profile was paying roughly half its text throughput to a default package, and the perf guard had the cost baked into its own baseline so nothing flagged it. #### Other info (issues closed, discussion etc) Findings C17 and C18 of the 5.0 QA sweep. Bisected there to ` |
||
|
Vadim Peretokin
|
c1d1f1aec8
|
fix: stop the telnet reader writing a NUL past the data it was given (#9677)
#### Brief overview of PR changes/additions * `cTelnet::processSocketData()` terminated its input at `in_buffer[amount + 1]`, one byte past the data it was given, and did so *before* checking the `-1`/`0` returns from `QIODevice::read()`. It now guards first, then terminates at `in_buffer[amount]`. * The guard is `amount <= 0` rather than `== -1`, because `loopbackTest()` narrows a `qsizetype` into an `int` and can produce a negative that is not `-1`. * Adds `cTelnetBufferTest` (5 slots) and drops the `reserve(size + 16)` slack that three existing telnet tests carried purely to absorb the stray write, which turns them into regression guards too. #### Motivation for adding to Mudlet The socket path survived this because `slot_socketReadyToBeRead()` over-allocates its stack buffer, but the same function is reached from Lua's `feedTelnet()` via `loopbackTest()`, which passes a `QByteArray` sized exactly to its contents - so the stray NUL landed one byte past a heap allocation. That is a real out-of-bounds write reachable from any script, and the workarounds already sitting in our test suite show it has been quietly worked around rather than fixed. #### Other info (issues closed, discussion etc) Closes #1065. Supersedes the closed #8438, which carried the same fix under 19 commits of unrelated history and had CI red on a faulty assertion in its own test. **Test case:** reverting the `src/ctelnet.cpp` hunk makes 3 of the 5 new slots fail and AddressSanitizer report `heap-buffer-overflow ... in cTelnet::processSocketData(char*, int, bool)`; restoring it gives 7 passed / 0 failed, and the full suite is 72/72 serially. One thing to flag for review: this adds `friend class cTelnetBufferTest;` to `cTelnet`, since `processSocketData()` is private and the public `loopbackTest()` cannot express a caller-laid-out buffer. It sits beside the existing `friend class TelnetTlsPromptTest;`, so there is precedent, but it is a test name in a shipped header and worth a second opinion. Three review findings were deliberately left out of scope. The MCCP decompression path hit the same overflow, via the re-entry that passes `remainingData`/`remainingAmount` back into `processSocketData()` - the fix covers it, but nothing under `test/` exercises compression at all, so it is fixed-but-unguarded and a dedicated MCCP test belongs in its own PR. The other two are pre-existing and orthogonal: a read error (`amount == -1`) is still silent, because `slot_socketError()` has been commented out as unused since 2017; and `mDecompressionRecursionDepth` is hand-balanced across four decrements rather than held by a scope guard (verified balanced today, but fragile). Happy to do any of them as a follow-up. Assisted-by: Claude:claude-opus-5 |
||
|
Vadim Peretokin
|
7d67d4bfb9
|
infrastructure: add performance baseline benchmark for the text and trigger pipeline (#9509)
#### Brief overview of PR changes/additions - Adds `test/functional_tests/PipelineBenchmark.cpp` - a headless, deterministic, report-only benchmark. It feeds a fixed 25,000-line corpus (plain text, ANSI SGR colour, UTF-8, and long wrapping-heavy prose adopted from the Stressinator display package) through the production `cTelnet -> TBuffer -> TConsole -> TriggerUnit` path via `loopbackTest()`, and prints `METRIC` lines: text-pipeline throughput, trigger-engine throughput with a realistic ~34-trigger set, derived trigger overhead, and peak RSS. - Adds `test/compare-perf-baseline.py` - the primary workflow. Given an older and a newer build run on the **same machine**, it parses their `METRIC` output, prints per-metric deltas, and exits non-zero on a PASS/FAIL against the 10% gate. - `test/functional_tests/CMakeLists.txt` builds the benchmark **always** but keeps it **out of the default ctest suite**; it is report-only and slow, so it should not run on every CI pipeline. Opt in with `-DREGISTER_PERF_BENCHMARK=ON` to also register it with ctest. - Adds `docs/libmudlet-perf-baseline.md` documenting the before/after workflow. **No canonical, machine-specific numbers are committed** - the figures in the doc are explicitly illustrative. #### Motivation for adding to Mudlet The libmudlet refactor's "no more than 10% throughput loss" gate is unenforceable without a reproducible way to measure it. Absolute numbers are meaningless across machines, so this provides a deterministic harness plus a same-machine before/after comparison tool that turns the gate into a mechanical PASS/FAIL. #### Other info (issues closed, discussion etc) Part of the libmudlet refactor (#8681, #9011) - referenced, not closed. - **Report-only**: it makes no timing assertions (absolute speed varies wildly between machines and CI runners), but it does assert the pipeline genuinely processed data - console buffer fill, every trigger compiled/registered, and an untimed sentinel trigger firing - so a silently-disconnected pipeline fails instead of reporting inflated numbers. - Each phase feeds the corpus 6 times and reports the **fastest pass**: the least-disturbed pass isolates intrinsic speed from transient CPU contention, keeping run-to-run spread ~2% even on a loaded machine. - **Companion, not a replacement, for the live-GUI path.** `PipelineBenchmark` runs offscreen and covers the telnet -> buffer -> trigger core (the piece the refactor moves). The **Stressinator display benchmark** covers the on-screen render/echo path on a live build; its wrapping-heavy prose has been adopted into this corpus. Between them they cover bytes-off-the-socket to pixels-on-screen. Assisted-by: Claude:claude-fable-5 **Test case:** Primary workflow - build an older and a newer tree on the same machine, then compare: ``` flock /tmp/mudlet-functional-tests.lock \ test/compare-perf-baseline.py --run \ ../mudlet-before/build/test/functional_tests/PipelineBenchmark \ build/test/functional_tests/PipelineBenchmark ``` Or run the benchmark once directly (it is built even without the ctest opt-in): ``` QT_QPA_PLATFORM=offscreen ASAN_OPTIONS=detect_leaks=0 \ ./build/test/functional_tests/PipelineBenchmark ``` Illustrative output (absolute values vary per machine; nothing is asserted on timing): ``` METRIC text_corpus_lines 25000 METRIC text_corpus_bytes 1436934 METRIC text_lines_per_sec 4281.46 METRIC text_mb_per_sec 0.41 METRIC text_best_pass_ms 5839.12 METRIC trigger_count 34 METRIC trigger_lines_per_sec 3323.25 METRIC trigger_mb_per_sec 0.32 METRIC trigger_best_pass_ms 7523.40 METRIC trigger_overhead_ms 1683.64 METRIC peak_rss_kb 1402384 ``` To drive it through ctest instead, configure with `-DREGISTER_PERF_BENCHMARK=ON`, then `ctest -R PipelineBenchmark -V`. |