From 0fa22eb5e0c3088cc02bf72506c7b3bcc1c11c21 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 09:18:16 -0700 Subject: [PATCH 1/5] perf(tpar): skip par() lookups for non-par names get_tpar() fell back to suppressWarnings(par(o)) for any name absent from .tpar. Most tpar parameters are tinyplot's own (grid.bg, palette, lty.xaxs, x/yaxr, ...), which base par() does not recognise, so it warned on each one and we paid to raise and immediately suppress that warning. A miss cost ~50us against ~13us for a real lookup, and a single plot takes that path around a dozen times. Gate the par() call behind the set of names base R actually knows, so unknown names fall straight through to the next candidate or the default. Same result, no warning round trip. The name set is cached in .tinyplot_env on first use rather than at load time: par() needs an open device and would otherwise open one as a side effect during .onLoad, writing a stray Rplots.pdf. The set does not vary by device (identical across pdf, png, svglite and postscript), so one per-session cache is safe. Benchmarked against v0.7.0 across 28 canonical plot calls: development HEAD was ~6% slower than the release, and is now ~9% faster. 300 plots in succession drop from 2130ms to 1777ms. Full suite passes, 729/729, with NOT_CRAN=true so snapshots ran. Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- R/environment.R | 3 ++- R/tpar.R | 30 ++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/R/environment.R b/R/environment.R index f6d66482c..962b90784 100644 --- a/R/environment.R +++ b/R/environment.R @@ -36,5 +36,6 @@ set_environment_variable( .saved_par_first = NULL, .last_call = NULL, .tpar_hooks = NULL, - .registered_themes = NULL + .registered_themes = NULL, + .base_par_names = NULL ) diff --git a/R/tpar.R b/R/tpar.R index 2d6530407..07524d6ec 100644 --- a/R/tpar.R +++ b/R/tpar.R @@ -215,17 +215,43 @@ tpar = function(..., hook = FALSE) { } +# Names that base par() recognises. Querying par() for anything else emits a +# warning, and raising then suppressing it costs about four times the lookup +# itself. Most tpar parameters are tinyplot's own (grid.bg, palette, x/yaxr, +# ...), so without this guard every plot pays that penalty a dozen times over. +# +# Cached in .tinyplot_env on first use rather than at load time: par() needs an +# open device, and calling it from .onLoad would open one as a side effect +# (writing a stray Rplots.pdf). The name set does not vary by device, so a +# single per-session cache is safe. +base_par_names = function() { + # read directly rather than via get_environment_variable(): this sits on a + # path hit ~24 times per plot, where the helper's overhead is measurable + bpn = .tinyplot_env[[".base_par_names"]] + if (is.null(bpn)) { + # read-only pars are absent from par(no.readonly = FALSE) but still + # valid to query + bpn = c( + names(par(no.readonly = FALSE)), + "cin", "cra", "csi", "cxy", "din", "page" + ) + set_environment_variable(.base_par_names = bpn) + } + return(bpn) +} + # Two levels of priority: .tpar[["name"]] -> par("name") get_tpar = function(opts, default = NULL, tpar_list = NULL) { if (is.null(tpar_list)) tpar_list = .tpar # parameter priority # .tpar[["name"]] -> par("name") + bpn = base_par_names() for (o in opts) { tp = tpar_list[[o]] if (!is.null(tp)) { return(tp) - } else { - p = suppressWarnings(par(o)) + } else if (o %in% bpn) { + p = par(o) if (!is.null(p)) { return(p) } From 886f1c777e4012bc61f10435727042585dd932a6 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 09:23:51 -0700 Subject: [PATCH 2/5] perf(assertions): build error messages only on failure The assert_* helpers composed their error message with sprintf() before running the check, so every passing assertion paid for a string it then threw away. Passing is the overwhelmingly common case: assert_tpar() alone runs about 30 assertions on every plot. Move each sprintf() inside the failure branch. This also defers the `name = as.character(substitute(x))` default, which is only referenced when composing the message, so callers that do not pass `name` explicitly no longer pay for substitute() on the happy path. Messages are unchanged. All 22 error paths across the touched helpers were compared before and after, including the substitute()-derived names, and are byte-identical. assert_tpar() drops from ~99us to ~77us (-22%) and assert_numeric() from ~3.6us to ~2.9us (-19%). End to end this is a modest ~0.6% on plot time, since the helpers are a small share of it; the 300-plot succession goes from 1777ms to 1737ms. Full suite passes, 729/729, with NOT_CRAN=true so snapshots ran. Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- R/assertions.R | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/R/assertions.R b/R/assertions.R index 564a8297c..2b8ed3438 100644 --- a/R/assertions.R +++ b/R/assertions.R @@ -40,9 +40,8 @@ check_true = function(x, null.ok = FALSE) { } assert_true = function(x, null.ok = FALSE, name = as.character(substitute(x))) { - msg = sprintf("`%s` must be true.", name) if (!isTRUE(check_true(x, null.ok = null.ok))) { - stop(msg, call. = FALSE) + stop(sprintf("`%s` must be true.", name), call. = FALSE) } } @@ -57,9 +56,8 @@ check_string = function(x, null.ok = FALSE) { } assert_string = function(x, null.ok = FALSE, name = as.character(substitute(x))) { - msg = sprintf("`%s` must be a string.", name) if (!isTRUE(check_string(x, null.ok = null.ok))) { - stop(msg, call. = FALSE) + stop(sprintf("`%s` must be a string.", name), call. = FALSE) } } @@ -74,9 +72,8 @@ check_flag = function(x, null.ok = FALSE) { } assert_flag = function(x, null.ok = FALSE, name = as.character(substitute(x))) { - msg = sprintf("`%s` must be a logical flag.", name) if (!isTRUE(check_flag(x, null.ok = null.ok))) { - stop(msg, call. = FALSE) + stop(sprintf("`%s` must be a logical flag.", name), call. = FALSE) } } @@ -146,8 +143,8 @@ assert_length = function(x, len = 1, null.ok = FALSE, name = as.character(substi if (is.null(x) && isTRUE(null.ok)) { return(invisible(TRUE)) } - msg = sprintf("`%s` must be one of these lengths: %s", name, paste(len, collapse = ", ")) if (!length(x) %in% len) { + msg = sprintf("`%s` must be one of these lengths: %s", name, paste(len, collapse = ", ")) stop(msg, call. = FALSE) } } @@ -165,8 +162,9 @@ assert_logical = function(x, null.ok = FALSE, name = as.character(substitute(x)) if (is.null(x) && isTRUE(null.ok)) { return(invisible(TRUE)) } - msg = sprintf("`%s` must be a logical vector", name) - if (!is.logical(x)) stop(msg, call. = FALSE) + if (!is.logical(x)) { + stop(sprintf("`%s` must be a logical vector", name), call. = FALSE) + } } @@ -197,9 +195,9 @@ assert_integerish = function(x, len = NULL, lower = NULL, upper = NULL, null.ok if (isTRUE(null.ok) && is.null(x)) { return(invisible()) } - msg = sprintf("`%s` must be integer-ish", name) if (is.null(x) && !isTRUE(null.ok)) stop(sprintf("%s should not be NULL.", name), call. = FALSE) if (!isTRUE(check_integerish(x, len = len, lower = lower, upper = upper, null.ok = null.ok))) { + msg = sprintf("`%s` must be integer-ish", name) if (!is.numeric(x)) msg = paste0(msg, "; it is not numeric") if (!is.null(len) && length(x) != len) msg = paste0(msg, sprintf("; its length must be %s", len)) if (!is.null(lower) && any(x < lower)) msg = paste0(msg, sprintf("; all values must be greater than or equal to %s", lower)) @@ -229,8 +227,8 @@ check_numeric = function(x, len = NULL, lower = NULL, upper = NULL, null.ok = TR } assert_numeric = function(x, len = NULL, lower = NULL, upper = NULL, null.ok = FALSE, name = as.character(substitute(x))) { - msg = sprintf("`%s` must be numeric", name) if (!isTRUE(check_numeric(x, len = len, lower = lower, upper = upper, null.ok = null.ok))) { + msg = sprintf("`%s` must be numeric", name) if (!is.null(len) && length(x) != len) msg = paste0(msg, sprintf("; its length must be %s", len)) if (!is.null(lower) && any(x < lower)) msg = paste0(msg, sprintf("; all values must be greater than or equal to %s", lower)) if (!is.null(upper) && any(x > upper)) msg = paste0(msg, sprintf("; all values must be less than or equal to %s", upper)) @@ -239,12 +237,17 @@ assert_numeric = function(x, len = NULL, lower = NULL, upper = NULL, null.ok = F } assert_data_frame = function(x, min_rows = 0, min_cols = 0, name = as.character(substitute(x))) { - msg = sprintf("`%s` must be a data.frame.", name) - if (!is.data.frame(x)) stop(msg, call. = FALSE) - msg = sprintf("Number of rows in `%s` must be at least `%s`", name, min_rows) - if (nrow(x) < min_rows) stop(msg, call. = FALSE) - msg = sprintf("Number of columns in `%s` must be at least `%s`", name, min_cols) - if (ncol(x) < min_cols) stop(msg, call. = FALSE) + if (!is.data.frame(x)) { + stop(sprintf("`%s` must be a data.frame.", name), call. = FALSE) + } + if (nrow(x) < min_rows) { + msg = sprintf("Number of rows in `%s` must be at least `%s`", name, min_rows) + stop(msg, call. = FALSE) + } + if (ncol(x) < min_cols) { + msg = sprintf("Number of columns in `%s` must be at least `%s`", name, min_cols) + stop(msg, call. = FALSE) + } } From d5511fad83ea0293f24090b0619c2246fb64f9e2 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 11:51:39 -0700 Subject: [PATCH 3/5] perf(utils): copy environments in one shot in env2env() env2env() assigned key by key. A single plot moves a few hundred keys across roughly 20 calls, and the per-key assign() dominates: swapping the loop for mget() + list2env() is about 4x faster on the copy itself. Behaviour is preserved. ifnotfound = list(NULL) keeps writing NULL for a key absent from the source (the loop did this via `[[`, which returns NULL rather than erroring), so the key is still created in the target. mget()'s inherits = FALSE default matches `[[` on an environment, so values in enclosing environments are still not picked up. list2env() returns the target environment visibly, so return invisible(NULL) to match what the for loop returned; no call site uses the value. Larger effect than the previous two commits: 9.7% off plot time, and every one of the 28 benchmarks improves. Cumulative for the branch, development HEAD was ~6% slower than v0.7.0 and is now ~19% faster (0.814x); against dev HEAD the branch is 0.765x. The 300-plot succession drops from 2130ms to 1605ms. Full suite passes, 729/729, with NOT_CRAN=true so snapshots ran. Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- R/utils.R | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/R/utils.R b/R/utils.R index c32b2cd39..983502db7 100644 --- a/R/utils.R +++ b/R/utils.R @@ -112,9 +112,15 @@ env2env = function(source_env, target_env, keys = NULL) { if (is.null(keys)) { keys = ls(source_env, all.names = TRUE) } - for (nm in keys) { - assign(nm, source_env[[nm]], envir = target_env) - } + ## copy in one shot rather than one assign() per key: a single plot moves a + ## few hundred keys across ~20 calls, where the loop is about 4x slower. + ## ifnotfound preserves the old behaviour of writing NULL for an absent key, + ## and mget()'s inherits = FALSE default matches `[[` on an environment. + list2env( + mget(keys, envir = source_env, ifnotfound = list(NULL)), + envir = target_env + ) + invisible(NULL) } From 48a1faa7548d9c08db150d7d6cb257325893c4cd Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 14:11:13 -0700 Subject: [PATCH 4/5] news --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 05f1bb31b..702579885 100644 --- a/NEWS.md +++ b/NEWS.md @@ -311,6 +311,10 @@ not 'at'`. The free-facet code path listed the eligible types by name, so - Gradient legends drawn below the plot (e.g. `legend = "bottom!"`) no longer ride up over the x-axis under dynamic themes. (#719 @grantmcdermott) +### Internals + +- Performance improvemnts. (#723 @grantmcdermott) + ## v0.7.0 **tinyplot** v0.7.0 is a big release with many new features, including major From 75efa4a463cbb3a2d9ee2d4f352e0de268683319 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 14:17:37 -0700 Subject: [PATCH 5/5] fix(tpar): drop redundant read-only par names from the cache par(no.readonly = FALSE) already returns the read-only parameters (cin, cra, csi, cxy, din, page); it is no.readonly = TRUE that excludes them. Appending them was a no-op that left six duplicates in the allowlist, and the comment asserted the opposite of the API's actual behaviour. The resulting set is unchanged (72 names, same unique membership), so behaviour is identical; this only removes the duplicates and corrects the comment. Spotted in review by Copilot on #723. Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- R/tpar.R | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/R/tpar.R b/R/tpar.R index 07524d6ec..e6e1e074d 100644 --- a/R/tpar.R +++ b/R/tpar.R @@ -229,12 +229,10 @@ base_par_names = function() { # path hit ~24 times per plot, where the helper's overhead is measurable bpn = .tinyplot_env[[".base_par_names"]] if (is.null(bpn)) { - # read-only pars are absent from par(no.readonly = FALSE) but still - # valid to query - bpn = c( - names(par(no.readonly = FALSE)), - "cin", "cra", "csi", "cxy", "din", "page" - ) + # no.readonly = FALSE is the full set, including the read-only pars + # (cin, cra, csi, cxy, din, page); those cannot be set but are valid to + # query, so they belong here + bpn = names(par(no.readonly = FALSE)) set_environment_variable(.base_par_names = bpn) } return(bpn)