perf(save_tt): hold table_string as lines across build phases - #666
Closed
EinMaulwurf wants to merge 1 commit into
Closed
perf(save_tt): hold table_string as lines across build phases#666EinMaulwurf wants to merge 1 commit into
EinMaulwurf wants to merge 1 commit into
Conversation
`lines_insert()` / `lines_drop*()` took and returned a single string, so every call `strsplit()`-ed the *entire, growing* `@table_string` on the way in and `paste(collapse)`-ed it on the way out. In the hot backends these are called dozens of times per build (per hline/vline chunk, per unique cell style, per note, per span, per CSS group) - O(N * L) string traffic that is pure overhead. See tt_save_audit.md §2.4 / §4.2. Add a line-oriented layer and use it in the hot loops: * `R/utils.R`: `lines_insert_vec()` / `lines_drop_vec()` / `lines_drop_between_vec()` / `lines_drop_consecutive_empty_vec()` operate on a character vector of lines. The original string-based helpers become thin split -> vec -> collapse wrappers, so every existing caller keeps working unchanged. `lines_insert_vec()` splits `new` into individual lines on insert so a chain of `*_vec()` calls is byte-for-byte identical to the equivalent chain of string round-trips (multi-line inserts must not stay lumped into one element, or a later marker lookup could match the whole block - e.g. the Typst footer template embeds its own notes marker). * `R/table_string_lines.R`: `table_string_lines()` getter + `table_string_lines<-` replacement that split / collapse `@table_string` exactly once, letting a backend hold the table as lines for a whole phase. * Typst (`R/typst_style.R`, `R/typst_tt.R`): `typst_hlines()`, `typst_vlines()`, `typst_apply_styles()` and `typst_notes()` now read the table as lines once, splice all their content via the cheap primitives, and write it back once. The same edits are applied in the same order. * HTML (`R/html_style.R`): the rowspan/colspan loop and the per-CSS-group loop do the same - one split / collapse per phase instead of one per item. Output is byte-for-byte identical: verified across 8 fixtures (simple, alignment, spans, heat-map, hlines/vlines, grouped columns, notes, combined) x 4 formats (typst/html/latex/markdown) = 32/32 identical to `main`, and the full tinytest suite passes (222 assertions, 0 failures; remaining files are gated on missing optional deps / maintainer-only environments).
Owner
|
thanks for this! the basic idea is good but the PR introduces to many superfluous helpers and code changes. I merged a simpler version here: #668 |
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
2 participants
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.
NOTE: This PR and the code changes are entirely "vibe coded" using GLM-5.2.
lines_insert()/lines_drop*()took and returned a single string, so every callstrsplit()-ed the entire, growing@table_stringon the way in andpaste(collapse)-ed it on the way out. In the hot backends these are called dozens of times per build (per hline/vline chunk, per unique cell style, per note, per span, per CSS group) - O(N * L) string traffic that is pure overhead.Add a line-oriented layer and use it in the hot loops:
R/utils.R:lines_insert_vec()/lines_drop_vec()/lines_drop_between_vec()/lines_drop_consecutive_empty_vec()operate on a character vector of lines. The original string-based helpers become thin split -> vec -> collapse wrappers, so every existing caller keeps working unchanged.lines_insert_vec()splitsnewinto individual lines on insert so a chain of*_vec()calls is byte-for-byte identical to the equivalent chain of string round-trips (multi-line inserts must not stay lumped into one element, or a later marker lookup could match the whole block - e.g. the Typst footer template embeds its own notes marker).R/table_string_lines.R:table_string_lines()getter +table_string_lines<-replacement that split / collapse@table_stringexactly once, letting a backend hold the table as lines for a whole phase.Typst (
R/typst_style.R,R/typst_tt.R):typst_hlines(),typst_vlines(),typst_apply_styles()andtypst_notes()now read the table as lines once, splice all their content via the cheap primitives, and write it back once. The same edits are applied in the same order.HTML (
R/html_style.R): the rowspan/colspan loop and the per-CSS-group loop do the same - one split / collapse per phase instead of one per item.Output is byte-for-byte identical: verified across 8 fixtures (simple, alignment, spans, heat-map, hlines/vlines, grouped columns, notes, combined) x 4 formats (typst/html/latex/markdown) = 32/32 identical to
main, and the full tinytest suite passes (222 assertions, 0 failures; remaining files are gated on missing optional deps / maintainer-only environments).