perf: avoid unnecessary deep copies from direct replacement function calls - #875
davidbudzynski wants to merge 49 commits into
Conversation
Replace direct calls to base replacement functions (attr<-, attributes<-, dim<-, dimnames<-) with equivalent assignment syntax, which avoids the forced deep copy that occurs when replacement functions are invoked directly. See fastverse#311.
- na_omit(): set the omit class with oldClass(x) <- before attaching - ffka(), setRnDF(), setRownames(): assignment syntax instead of direct attributes<-/attr<- calls - fdapply(), colsubset(): drop redundant attribute stripping before lapply()/vapply(), which ignore object-level attributes anyway See fastverse#311.
Build the GRP object into a local variable and set its class with oldClass(g) <- "GRP" instead of wrapping the list literal in a direct oldClass<- call, and use names(x) <- / attributes(x) <- assignment syntax for the group names and retained order attributes. See fastverse#311.
Construct the GRP list in a local variable, assign group names with names(x) <- and set the class with oldClass(x) <- "GRP" instead of direct oldClass<-/names<- replacement calls. See fastverse#311.
- fgroup_by: assign names on the name-lookup list with names(x) <- - names<-.GRP_df: unclass, rename, then restore the saved class - fgroup_vars: build named indices/logicals with assignment syntax instead of direct names<-/[<- replacement calls - as_factor_qG/qG: set attributes with attributes(x) <- instead of direct attributes<- calls See fastverse#311.
Replace direct names<-/dimnames<- replacement calls on freshly computed results with the equivalent assignment form, avoiding forced deep copies of the result vectors/matrices. See fastverse#311.
Replace direct names<-/dimnames<- replacement calls on freshly computed results with the equivalent assignment form, avoiding forced deep copies. See fastverse#311.
- get_vars_ind/get_vars_indl/fselect: build named indices and logical vectors with assignment syntax instead of direct names<-/[<-/attr<- replacement calls - get_vars_ind<-: restore the class with oldClass(x) <- before the conditional data.table allocation check - date_vars: drop redundant attribute stripping before vapply() See fastverse#311.
- fsubset/ftransform/fmutate helpers: use names(x) <- and oldClass(x) <- assignment syntax instead of direct names<-/oldClass<-[<- calls - setup_across: set the class on a shallow copy so the unclassed data returned in the result stays untouched - fFUN internals: name results with assignment syntax See fastverse#311.
Use oldClass(x) <- and attributes(x) <- assignment syntax instead of direct oldClass<-/attributes<- replacement calls in replace_na, replace_outliers and pad. See fastverse#311.
Build named statistics and the attrib attribute with assignment syntax instead of direct names<-/attr<- replacement calls. See fastverse#311.
The fsd.default/fvar.default refactors accidentally swapped the stable.algo flag in the grouped vector paths (fsd must use TRUE, fvar FALSE). Restores the exact original .Call arguments.
Re-adds the explanatory notes about shallow/deep copy behavior on the GRP order attribute and documents why flm strips dimnames for the qr method only.
|
Thanks @davidbudzynski! @claude can we also check whether lapply(res, function(l) {
names(l) <- rn
l
})is really an improvement over res <- lapply(res, `names<-`, rn)Please test using different data sizes. |
|
Claude finished @SebKrantz's task in 3m 34s —— View job Task: benchmark
|
Benchmarks requested in fastverse#875 (R 4.6.1, macOS arm64) show lapply(res, `names<-`, rn) is ~2x faster than the closure form at all tested sizes (5-500 elems x 10-1e6 rows), and tracemem/profmem confirm that neither form duplicates element data through lapply on R >= 4.x, so the complex-assignment rewrite only added interpreter overhead (~150 ns/elem) with no memory benefit.
…rk audit Systematic benchmarks on R 4.6.1 (macOS arm64) show that direct calls to base replacement primitives are performance-equivalent to assignment syntax in every context occurring in this sweep: neither form duplicates data through lapply/apply-family callbacks, function arguments, or inline-computed temporaries (collapse requires R >= 4.1, so reference counting semantics apply throughout). Closure rewrites inside iteration add interpreter overhead (~150 ns/element), and removing the attribute strip before vapply() in date_vars() silently added column names to two documented return modes. Whole-function dev-vs-PR comparisons (fmean/fsum/collap/B/W/fmutate over nrow 1e4-1e6 x ncol 10-50 x groups 10-500) were statistically indistinguishable, and all 10 functional snapshots matched byte-for-byte. Full test suite: 12,683 passes / 0 failures / 10 pre-existing skips. Kept (verified behaviorally identical, strictly less work): - fdapply(): drop attribute strip before lapply() - colsubset(): drop attribute strip before vapply() See fastverse#875 and fastverse#311 for the full audit.
|
@SebKrantz Benchmark results for your question (R 4.6.1, macOS arm64; min of 5 auto-sized batches per size, exact allocations via
the direct call is consistently ~2× faster at every size tested (5–500 elements × 10–10⁶ rows), and I've reverted |
|
Following up on the review discussion above: after systematically benchmarking every rewrite pattern in this PR, I've rolled the sweep back to just two changes that demonstrably help (04333c6). What exactly was benchmarkedEach old form was compared head-to-head with its PR rewrite:
How
Findings
What was keptVerified behaviorally identical and strictly less work: removing the redundant Net branch diff vs Scripts & raw data: https://gist.github.com/davidbudzynski/9af71984f7b06b4ee459f25c69833e88 Full archetype results (µs/call · bytes/call · tracemem copies)
A5 shows both forms shallow-copy the frame identically when reclassing through an argument — assignment syntax buys nothing there either. End-to-end dev-vs-PR timing deviations (%)
Bold cells exceed ±5%; all are small-data cases where absolute deltas are single-digit microseconds, and functions untouched by the PR show the same scatter (run noise). |
|
Thanks @davidbudzynski! I just tested this though and find: library(collapse)
#> collapse 2.1.7, see ?`collapse-package` or ?`collapse-documentation`
#>
#> Attaching package: 'collapse'
#> The following object is masked from 'package:stats':
#>
#> D
library(bench)
duplAttributes <- collapse:::duplAttributes
fdapply <- function(X, FUN, ...) duplAttributes(lapply(`attributes<-`(X, NULL), FUN, ...), X)
fdapply2 <- function(X, FUN, ...) duplAttributes(lapply(X, FUN, ...), X)
df <- qDF(lapply(rep(10000,1000), rnorm))
mark(fdapply(df, identity), fdapply2(df, identity))
#> # A tibble: 2 × 6
#> expression min median `itr/sec` mem_alloc `gc/sec`
#> <bch:expr> <bch:tm> <bch:tm> <dbl> <bch:byt> <dbl>
#> 1 fdapply(df, identity) 141µs 164µs 5919. 7.86KB 62.0
#> 2 fdapply2(df, identity) 147µs 167µs 5934. 7.86KB 65.1
df <- qDF(lapply(rep(100000,100), rnorm))
mark(fdapply(df, identity), fdapply2(df, identity))
#> # A tibble: 2 × 6
#> expression min median `itr/sec` mem_alloc `gc/sec`
#> <bch:expr> <bch:tm> <bch:tm> <dbl> <bch:byt> <dbl>
#> 1 fdapply(df, identity) 14.7µs 17.2µs 57083. 848B 68.6
#> 2 fdapply2(df, identity) 15.9µs 18.3µs 52384. 848B 62.9
df <- qDF(lapply(rep(1000,10000), rnorm))
mark(fdapply(df, identity), fdapply2(df, identity))
#> # A tibble: 2 × 6
#> expression min median `itr/sec` mem_alloc `gc/sec`
#> <bch:expr> <bch:tm> <bch:tm> <dbl> <bch:byt> <dbl>
#> 1 fdapply(df, identity) 1.41ms 1.57ms 632. 78.2KB 82.8
#> 2 fdapply2(df, identity) 1.48ms 1.64ms 607. 78.2KB 79.3Created on 2026-08-27 with reprex v2.1.1 The reasons appears to be that So yeah, really sorry if that leaves this PR void, but I highly appreciate the effort and believe we can close #311 now – it is very good to know that all the memory issues have been addressed and we are really not making any mistakes as far as based R programming in collapse is concerned. Perhaps also #311 was resolved by imrovements to the R source code itself. R has become a lot more memory efficient over the years. |
|
Closed for now... |
2 participants
Description
Fixes #311.
As described in the issue, calling base R replacement functions directly (e.g.
`oldClass<-`(x, "cls")) entails a deep copy in some cases, while the equivalent assignment syntax (oldClass(x) <- "cls") does not. This PR rewrites all remaining direct replacement-function call sites inR/to use assignment syntax. Semantics are fully preserved — only unnecessary allocations are removed.Main Changes
Replacement calls converted to assignment syntax
All active call sites (~175 across 35 files in
R/) of the following base replacement primitives were converted:`oldClass<-`/`class<-`return(\oldClass<-`(l, "GRP"))`oldClass(l) <- "GRP"; return(l)`attributes<-`\`attributes<-\`(x, ax)attributes(x) <- ax; return(x)`attr<-`setRnDF <- function(df, nm) \`attr<-\`(df, "row.names", nm){ attr(df, "row.names") <- nm; df }`names<-`/`dimnames<-`return(\names<-`(res, lev))`names(res) <- lev; return(res)`dim<-`\`dim<-\`(outer(...), NULL)res <- outer(...); dim(res) <- NULL`[<-`\`[<-\`(logical(n), ind, TRUE)out <- logical(n); out[ind] <- TRUEFiles covered include the core grouping machinery (
GRP.R,BY.R,qG/qFconversions), all grouped statistical functions (fsum,fmean,fsd/fvar,fmin/fmax,fprod,fnobs,ffirst/flast,fmode,fnth/fmedian,varying), data manipulation (fsubset/ftransform/fmutate/across,fselect/get_vars*,recode_replace,roworder/colorder,join,rsplit), quick conversions (qDF/qDT/qM/unattrib), and summary/printing methods (descr,qsu,psmat,pwcor/pwcov,psacf,unlist2d,flm,indexing,collap,dapply,fcount,B/W/HDB/HDW/STD).Related clean-ups found during the sweep
lapply()/vapply()infdapply(),colsubset()anddate_vars()— these iterate elements and ignore object-level attributes, so stripping only caused an extra copysetup_across()now sets the class on a shallow copy (dsub <- d; oldClass(dsub) <- pe$cld) so the unclassed data returned in the result stays untouched — a comment explains why mutatingddirectly would be observableorderattribute copy-semantics note,flm()'s dimnames handling for theqrmethod)Intentionally left untouched
misc/legacy/`ftransform<-`,`get_vars_ind<-`,`add_vars<-`, ...): these are R-defined closures, not base primitives, and are not subject to the forced-copy behavior described in Optimize class assignments #311src/C code: attribute handling there uses different mechanisms entirelyVerification
developmentbranch: the same 8 pre-existing environment-related errors on both, zero new failures (verified by buildingdevelopmentinto a separate library and diffing test outcomes).Call()argument vectors in modified files were diffed byte-for-byte againstdevelopment(this caught and fixed an accidentally flippedstable.algoflag during development — see commit history)fgroup_by(mtcars, cyl, vs2 = vs, am)renaming,findex_by()renaming,qtab()auto dimension naming,colorder(pos = "exchange"),na_omit(na.attr = TRUE)Checklist
Notes on the checklist:
[<-sites (infgroup_by,findex_by,qtabandposord) which were converted in follow-up commitssetup_across, GRP order attribute,flm, redundant-strip removals); mechanical one-to-one rewrites are left uncommented to keep the diff readable.Rdupdates were applicableAdditional Context
Benchmark evidence
Allocations per call measured with
Rprofmem()(R 4.6.1, macOS arm64), comparing the direct-call form with the equivalent assignment form used in this PR:names<-on freshly computed grouped resultdimnames<-on grouped matrix resultattributes<-strip beforelapply()oldClass<-on freshly built listattributes<-factor conversionThe largest wins are on hot paths that name small grouped results (
use.g.names = TRUE), where the direct-call form duplicated the entire result vector.