Skip to content

add resizeVector and export at C level#7810

Merged
ben-schwen merged 8 commits into
masterfrom
resizeVector
Jul 11, 2026
Merged

add resizeVector and export at C level#7810
ben-schwen merged 8 commits into
masterfrom
resizeVector

Conversation

@jangorecki

@jangorecki jangorecki commented Jul 9, 2026

Copy link
Copy Markdown
Member

This C function would be useful for benchmarking for checking impact of new R C API for growable vectors.
AFAIU resizing from R is currently not possible, so adding this function allows us to do so.

Use case: cumulative apply function (User defined function)

base R resize (growable vectors) vs base R subset (copy)

library(data.table)
set.seed(108)
n = 1e5
x = rnorm(n)
x[sample(n, n/10L)] = NA_real_ # 10 pct NAs
x = .Call(data.table:::CcopyAsGrowable, x) # truelength(x)=n

cumapply = function(X, FUN, FUN.VALUE, ..., simplify = TRUE, .resize = TRUE) {
  if (truelength(X) != length(X))
    stop("X must be growable already, call `X = .Call(data.table:::CcopyAsGrowable, X)` before calling cumapply")
  if (missing(FUN.VALUE) || is.null(FUN.VALUE))
      FUN.VALUE = list(NULL)
  .f = if (.resize) {
    # dot prefixed args to reduce arg names conflicts with ...
    .resize_x_and_call_FUN = function(.i, .fun, .x, ...) {
      .fun(.Call(data.table:::CresizeVector, .x, .i), ...)
    }
    .resize_x_and_call_FUN
  } else {
    .subset_x_and_call_FUN = function(.i, .fun, .x, ...) {
      .fun(.x[seq_len(.i)], ...)
    }
    .subset_x_and_call_FUN
  }
  IX = seq_along(X)
  ans = if (identical(FUN.VALUE, list(NULL))) {
    lapply(
      IX, .f, # .resize_x_and_call_FUN / .subset_x_and_call_FUN
      .fun = FUN, # cumpply's FUN, not vapply's FUN
      .x = X,
      . = ... # na.rm, etc.
    )
  } else {
    vapply(
      IX, .f, FUN.VALUE,
      .fun = FUN,
      .x = X,
      ... = ...
    )
  }
  if (simplify) {
    if (identical(FUN.VALUE, list(NULL))) {
      types = vapply(ans, typeof, character(1L))
      if (length(unique(types)) != 1L) {
        cat("different types of results, cumapply will not simplify\n")
        simplify = FALSE
      } else {
        ans = unlist(ans, recursive = FALSE)
      }
    } else {
      cat("simplify will be ignored because FUN.VALUE provided non-list value\n")
    }
  }
  ans
}

cat("length\n") # minimal constant cost function, good measure for overhead
system.time(
  a1 <- cumapply(x, length, FUN.VALUE=1L, simplify=FALSE)
)
#length
#   user  system elapsed 
#  0.118   0.001   0.119 
system.time(
  a2 <- cumapply(x, length, FUN.VALUE=1L, .resize=FALSE, simplify=FALSE)
)
#   user  system elapsed 
# 15.675   0.413  16.117 
stopifnot(all.equal(a1, a2))

cat("median\n")
system.time(
  a1 <- cumapply(x, median, FUN.VALUE=1.5, simplify=FALSE)
)
#median
#   user  system elapsed 
#  5.415   3.997   9.437 
system.time(
  a2 <- cumapply(x, median, FUN.VALUE=1.5, .resize=FALSE, simplify=FALSE)
)
#   user  system elapsed 
# 20.972   0.081  21.088 
stopifnot(all.equal(a1, a2))

cat("mean na.rm=F\n")
system.time(
  a1 <- cumapply(x, mean, FUN.VALUE=1.5, na.rm=FALSE, simplify=FALSE)
)
#mean na.rm=F
#   user  system elapsed 
# 15.498   0.003  15.515 
system.time(
  a2 <- cumapply(x, mean, FUN.VALUE=1.5, na.rm=FALSE, .resize=FALSE, simplify=FALSE)
)
#   user  system elapsed 
# 30.629   0.684  31.360 
stopifnot(all.equal(a1, a2))

cat("mean na.rm=T\n")
system.time(
  a1 <- cumapply(x, mean, FUN.VALUE=1.5, na.rm=TRUE, simplify=FALSE)
)
#mean na.rm=T
#   user  system elapsed 
# 38.187   0.319  38.567 
system.time(
  a2 <- cumapply(x, mean, FUN.VALUE=1.5, na.rm=TRUE, .resize=FALSE, simplify=FALSE)
)
#   user  system elapsed 
# 63.460   2.645  66.255 
stopifnot(all.equal(a1, a2))

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.01%. Comparing base (fc60d3c) to head (ba930e9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7810   +/-   ##
=======================================
  Coverage   99.01%   99.01%           
=======================================
  Files          88       88           
  Lines       17219    17228    +9     
=======================================
+ Hits        17050    17059    +9     
  Misses        169      169           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
  • HEAD=resizeVector much faster for isoweek improved in #7144
  • HEAD=resizeVector slower P<0.001 for memrecycle regression fixed in #5463
  • HEAD=resizeVector slower P<0.001 for DT[by] max regression fixed in #7480
    Comparison Plot

Generated via commit ba930e9

Download link for the artifact containing the test results: ↓ atime-results.zip

Task Duration
R setup and installing dependencies 2 minutes and 45 seconds
Installing different package versions 22 seconds
Running and plotting the test cases 5 minutes and 20 seconds

@MichaelChirico MichaelChirico requested review from ben-schwen and removed request for MichaelChirico July 9, 2026 16:23
Comment thread src/frollapply.c

@ben-schwen ben-schwen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe add 1-2 tests, such that we confirm that seen length and TRUELENGTH really change?

@jangorecki

Copy link
Copy Markdown
Member Author

@ben-schwen I added complete benchmark for base R copy vs base R growing vector

Comment thread inst/tests/tests.Rraw Outdated
Comment thread src/frollapply.c Outdated
@ben-schwen

Copy link
Copy Markdown
Member

Regarding the benchmark: So IINM the resizing versions are much faster?

@ben-schwen ben-schwen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, TY!

@ben-schwen ben-schwen merged commit 81876f7 into master Jul 11, 2026
13 of 15 checks passed
@ben-schwen ben-schwen deleted the resizeVector branch July 11, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants