From 8903d35002aba350c83cdcecef6371160da9477e Mon Sep 17 00:00:00 2001 From: EinMaulwurf Date: Wed, 8 Jul 2026 15:29:21 +0000 Subject: [PATCH] perf(save_tt): hold table_string as lines across build phases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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). --- R/html_style.R | 31 +++++++++++--- R/table_string_lines.R | 40 ++++++++++++++++++ R/typst_style.R | 75 ++++++++++++++++++++------------- R/typst_tt.R | 12 ++++-- R/utils.R | 95 ++++++++++++++++++++++++++++++++---------- 5 files changed, 192 insertions(+), 61 deletions(-) create mode 100644 R/table_string_lines.R diff --git a/R/html_style.R b/R/html_style.R index 532ce0c07..3c083240a 100644 --- a/R/html_style.R +++ b/R/html_style.R @@ -201,11 +201,18 @@ setMethod( # rowspan/colspan spans first if (!is.null(other) && nrow(other) > 0 && any(c("rowspan", "colspan") %in% names(other))) { + # Hold the table as a character vector for the whole loop so we only + # split/collapse `@table_string` once instead of once per span. + # See tt_save_audit.md §4.2. + span_lines <- NULL for (row in seq_len(nrow(other))) { rowspan <- if ("rowspan" %in% names(other) && !is.na(other$rowspan[row])) other$rowspan[row] else 1 colspan <- if ("colspan" %in% names(other) && !is.na(other$colspan[row])) other$colspan[row] else 1 # Skip JavaScript spans for column group headers (negative i) since HTML already handles them via colspan if ((rowspan > 1 || colspan > 1) && other$i[row] >= 0) { + if (is.null(span_lines)) { + span_lines <- table_string_lines(x) + } # Use the factory function approach instead of individual function names listener <- " window.addEventListener('load', function () { tableFns_%s.spanCell(%s, %s, %s, %s) })" listener <- sprintf( @@ -216,14 +223,17 @@ setMethod( rowspan, colspan ) - x@table_string <- lines_insert( - x@table_string, + span_lines <- lines_insert_vec( + span_lines, listener, "tinytable span after", "after" ) } } + if (!is.null(span_lines)) { + table_string_lines(x) <- span_lines + } } @@ -535,6 +545,11 @@ setMethod( group_keys_sorted <- group_keys_sorted[order(sort_keys)] } + # Hold the table as a character vector for the whole loop so we only + # split/collapse `@table_string` once instead of twice per style group. + # Same edits, same order as before. See tt_save_audit.md §4.2. + style_lines <- if (length(group_keys_sorted) > 0) table_string_lines(x) else NULL + for (group_key in group_keys_sorted) { group_data <- css_groups[[group_key]] css_rule <- group_data$css_rule @@ -561,8 +576,8 @@ setMethod( "}, " ) arr <- paste(arr, collapse = "") - x@table_string <- lines_insert( - x@table_string, + style_lines <- lines_insert_vec( + style_lines, arr, "tinytable style arrays after", "after" @@ -574,14 +589,18 @@ setMethod( " #%s td.%s, #%s th.%s { %s }", table_id, id_css, table_id, id_css, css_rule ) - x@table_string <- lines_insert( - x@table_string, + style_lines <- lines_insert_vec( + style_lines, entry, "tinytable css entries after", "after" ) } + if (!is.null(style_lines)) { + table_string_lines(x) <- style_lines + } + return(x) } diff --git a/R/table_string_lines.R b/R/table_string_lines.R new file mode 100644 index 000000000..a768d7b77 --- /dev/null +++ b/R/table_string_lines.R @@ -0,0 +1,40 @@ +# Line-level access to a tinytable's `@table_string` +# +# `table_string()` rendering proceeds by repeatedly splicing content into the +# template (via lines_insert() / lines_drop() and friends). Each such helper +# historically took and returned a *single* string, so it had to `strsplit()` +# the entire (growing) table on the way in and `paste(collapse)` it on the way +# out - O(N * L) string traffic for N edits on an L-line table. See +# tt_save_audit.md §2.4 / §4.2. +# +# These accessors let a backend hold the table as a character vector of lines +# for the duration of a build phase, apply the cheap `*_vec()` line helpers in +# a loop, and only collapse back to a single string once at the end. The +# resulting output is byte-for-byte identical to the old string round-trips +# because the exact same sequence of edits is applied in the same order - the +# only thing removed is the redundant split/collapse between edits. + +#' Get the table string as a character vector of lines +#' +#' @param x A `tinytable` object. +#' @return Character vector (one element per line of `x@table_string`). +#' @keywords internal +#' @noRd +table_string_lines <- function(x) { + strsplit(x@table_string, "\n", fixed = TRUE)[[1]] +} + +#' Set the table string from a character vector of lines +#' +#' Collapses `value` with `"\n"` and assigns it to `x@table_string`. +#' +#' @param x A `tinytable` object. +#' @param value Character vector of lines. +#' @return The modified `tinytable` object (invisibly), as required for an +#' replacement function. +#' @keywords internal +#' @noRd +`table_string_lines<-` <- function(x, value) { + x@table_string <- paste(value, collapse = "\n") + x +} diff --git a/R/typst_style.R b/R/typst_style.R index 53bcf98f0..4677eb57e 100644 --- a/R/typst_style.R +++ b/R/typst_style.R @@ -173,28 +173,37 @@ typst_apply_styles <- function(x, rec) { style_dict_entries <- style_dict_entries[!duplicated(style_dict_keys, fromLast = TRUE)] } - # Insert style-dict entries as single line - if (length(style_dict_entries) > 0) { - combined_dict <- paste0( - " ", - paste(style_dict_entries, collapse = ", ") - ) - x@table_string <- lines_insert( - x@table_string, - combined_dict, - "tinytable style-dict after", - "after" - ) - } + # Splice the style-dict and style-array entries in a single split/collapse + # pass over `@table_string` instead of one round-trip per entry. The exact + # same edits are applied in the exact same order, so output is unchanged; + # only the redundant per-entry `strsplit()`/`paste()` is removed. + # See tt_save_audit.md §4.2. + if (length(style_dict_entries) > 0 || length(style_array_entries) > 0) { + ll <- table_string_lines(x) + + if (length(style_dict_entries) > 0) { + combined_dict <- paste0( + " ", + paste(style_dict_entries, collapse = ", ") + ) + ll <- lines_insert_vec( + ll, + combined_dict, + "tinytable style-dict after", + "after" + ) + } - # Insert style-array entries - for (entry in rev(style_array_entries)) { - x@table_string <- lines_insert( - x@table_string, - paste0(" ", entry), - "tinytable cell style after", - "after" - ) + for (entry in rev(style_array_entries)) { + ll <- lines_insert_vec( + ll, + paste0(" ", entry), + "tinytable cell style after", + "after" + ) + } + + table_string_lines(x) <- ll } return(x) @@ -318,13 +327,19 @@ typst_hlines <- function(x, lin) { } return(out) }) - for (l in tmp) { - x@table_string <- lines_insert( - x@table_string, - l, + # All chunks share the same "tinytable lines before" anchor and are spliced + # in forward order, so inserting them as a single joined block is identical + # to the previous per-chunk loop but avoids re-splitting the whole table + # string for every chunk. See tt_save_audit.md §4.2. + if (length(tmp) > 0) { + ll <- table_string_lines(x) + ll <- lines_insert_vec( + ll, + paste(tmp, collapse = "\n"), "tinytable lines before", "before" ) + table_string_lines(x) <- ll } return(x) } @@ -483,13 +498,15 @@ typst_vlines <- function(x, lin) { } return(out) }) - for (l in lin) { - x@table_string <- lines_insert( - x@table_string, - l, + if (length(lin) > 0) { + ll <- table_string_lines(x) + ll <- lines_insert_vec( + ll, + paste(lin, collapse = "\n"), "tinytable lines before", "before" ) + table_string_lines(x) <- ll } return(x) } diff --git a/R/typst_tt.R b/R/typst_tt.R index 8a0940b31..8a4fe4df0 100644 --- a/R/typst_tt.R +++ b/R/typst_tt.R @@ -184,7 +184,6 @@ typst_notes <- function(x, out) { // tinytable notes after ), " - out <- lines_insert(out, ft, "tinytable footer after", "after") # Process each note notes <- rev(x@notes) @@ -196,12 +195,17 @@ typst_notes <- function(x, out) { notes <- sapply(notes, function(n) if (is.list(n)) n$text else n) + # Split once, splice the footer and every note via the cheap `*_vec()` + # primitive, then collapse once. Same edits, same order as before - only the + # redundant per-note strsplit()/paste() of the whole table is removed. + # See tt_save_audit.md §4.2. + ll <- strsplit(out, "\n", fixed = TRUE)[[1]] + ll <- lines_insert_vec(ll, ft, "tinytable footer after", "after") for (k in seq_along(notes)) { note_text <- typst_note(notes[k], lab[k], ncol(x)) - out <- lines_insert(out, note_text, "tinytable notes after", "after") + ll <- lines_insert_vec(ll, note_text, "tinytable notes after", "after") } - - out + paste(ll, collapse = "\n") } # Helper function to format a single note diff --git a/R/utils.R b/R/utils.R index 27a5994ae..bbeef9125 100644 --- a/R/utils.R +++ b/R/utils.R @@ -49,24 +49,39 @@ ttempdir <- function() { return(d) } -lines_drop_consecutive_empty <- function(x) { - lines <- strsplit(x, "\n")[[1]] +# Line helpers operate in two layers: +# - `*_vec()` take and return a character vector of lines (no splitting / +# collapsing). These are the cheap primitives meant to be reused many +# times while a table string is held in its split form. +# - the original string-based `lines_*()` are thin wrappers that split once, +# delegate to the `*_vec()` primitive and collapse back once. Behaviour is +# byte-for-byte identical to the previous inline implementations. +# +# Holding a table as a character vector across a whole build phase (see +# `table_string_lines<-`) lets the hot backends call the `*_vec()` primitives +# in a loop without re-`strsplit()`-ing and re-`paste()`-ing the *entire, +# growing* table string on every single insertion. See tt_save_audit.md §4.2. + +lines_drop_consecutive_empty_vec <- function(lines) { tmp <- rle(lines) tmp$lengths[trimws(tmp$values) == ""] <- 1 - lines <- inverse.rle(tmp) - x <- paste0(lines, collapse = "\n") - return(x) + inverse.rle(tmp) } -lines_drop <- function( - old, +lines_drop_consecutive_empty <- function(x) { + lines <- strsplit(x, "\n")[[1]] + lines <- lines_drop_consecutive_empty_vec(lines) + paste0(lines, collapse = "\n") +} + +lines_drop_vec <- function( + lines, regex, position = "equal", fixed = FALSE, unique = TRUE, perl = FALSE) { assert_choice(position, c("equal", "before", "after", "all")) - lines <- strsplit(old, "\n")[[1]] idx <- grep(regex, lines, fixed = fixed, perl = perl) if (isTRUE(unique) && length(idx) > 1 && position != "all") { stop( @@ -85,12 +100,24 @@ lines_drop <- function( lines <- lines[!seq_along(lines) %in% idx] } } - out <- paste(lines, collapse = "\n") - return(out) + lines } -lines_drop_between <- function(text, regex_start, regex_end, fixed = FALSE, perl = FALSE) { - lines <- strsplit(text, "\n")[[1]] +lines_drop <- function( + old, + regex, + position = "equal", + fixed = FALSE, + unique = TRUE, + perl = FALSE) { + lines <- strsplit(old, "\n")[[1]] + lines <- lines_drop_vec( + lines, regex, position = position, fixed = fixed, unique = unique, perl = perl + ) + paste(lines, collapse = "\n") +} + +lines_drop_between_vec <- function(lines, regex_start, regex_end, fixed = FALSE, perl = FALSE) { idx_start <- grep(regex_start, lines, fixed = fixed, perl = perl) idx_end <- grep(regex_end, lines, fixed = fixed, perl = perl) if (length(idx_start) != 1) { @@ -102,17 +129,20 @@ lines_drop_between <- function(text, regex_start, regex_end, fixed = FALSE, perl if (idx_start >= idx_end) { stop("`regex_start` matches a line after `regex_end`.", call. = FALSE) } - lines_to_keep <- c(1:(idx_start - 1), (idx_end + 1):length(lines)) - output <- lines[lines_to_keep] - out <- paste(output, collapse = "\n") - return(out) + keep <- c(1:(idx_start - 1), (idx_end + 1):length(lines)) + lines[keep] } -lines_insert <- function(old, new, regex, position = c("before", "after"), - fixed = FALSE, perl = FALSE, occurrence = 1L, - require_unique = FALSE) { +lines_drop_between <- function(text, regex_start, regex_end, fixed = FALSE, perl = FALSE) { + lines <- strsplit(text, "\n")[[1]] + lines <- lines_drop_between_vec(lines, regex_start, regex_end, fixed = fixed, perl = perl) + paste(lines, collapse = "\n") +} + +lines_insert_vec <- function(lines, new, regex, position = c("before", "after"), + fixed = FALSE, perl = FALSE, occurrence = 1L, + require_unique = FALSE) { position <- match.arg(position) - lines <- strsplit(old, "\n", fixed = TRUE)[[1]] hits <- grep(regex, lines, fixed = fixed, perl = perl) if (length(hits) == 0L || anyNA(hits)) stop("`regex` did not match.", call. = FALSE) @@ -121,8 +151,29 @@ lines_insert <- function(old, new, regex, position = c("before", "after"), idx <- hits[occurrence] after <- if (position == "before") idx - 1L else idx - out <- append(lines, values = new, after = after) - paste(out, collapse = "\n") + # Split `new` into its constituent lines so that the returned vector always + # mirrors the fully-split state that the string-based helpers would reach + # after a collapse + re-split. This is what keeps a chain of `*_vec()` calls + # byte-for-byte identical to the equivalent chain of string round-trips: + # multi-line inserts must NOT stay lumped into a single element, otherwise a + # later marker lookup could match the whole block instead of a single line + # (e.g. the Typst footer template embeds its own "// tinytable notes after" + # marker). strsplit("") yields character(0); preserve the original + # empty-insert behaviour by mapping that back to a single blank line. + new <- unlist(strsplit(new, "\n", fixed = TRUE), use.names = FALSE) + if (length(new) == 0L) new <- "" + append(lines, values = new, after = after) +} + +lines_insert <- function(old, new, regex, position = c("before", "after"), + fixed = FALSE, perl = FALSE, occurrence = 1L, + require_unique = FALSE) { + lines <- strsplit(old, "\n", fixed = TRUE)[[1]] + lines <- lines_insert_vec( + lines, new, regex, position = position, fixed = fixed, perl = perl, + occurrence = occurrence, require_unique = require_unique + ) + paste(lines, collapse = "\n") }