Skip to content

speed up bounds-checked element access and cache vector lengths - #1511

Open
kevinushey wants to merge 2 commits into
masterfrom
feature/bounds-check-perf
Open

kevinushey wants to merge 2 commits into
masterfrom
feature/bounds-check-perf

Conversation

@kevinushey

Copy link
Copy Markdown
Contributor

Closes #1510.

The per-element bounds check added in #1310 was noticeably slowing down element access in tight loops, including sugar expressions. This PR keeps the check (and its warning) but makes it cheap:

  • inst/include/Rcpp/vector/traits.h: the warning now lives in a noinline / cold helper (warn_index_out_of_bounds()), so tinyformat and the 8 KB message buffer are no longer inlined into every loop body that reads an element. proxy_cache (used by CharacterVector, List, ExpressionVector) now caches the vector length in update() rather than calling p->size() on every access.
  • inst/include/Rcpp/vector/Vector.h: size() / length() return the length cached by update() instead of calling Rf_xlength() (an opaque call into libR) each time. update() already runs on every set__(), so the cache is refreshed whenever the underlying SEXP changes.
  • inst/include/Rcpp/vector/Matrix.h, SubMatrix.h: cache the number of columns alongside the existing nrows, so ncol() / cols() (and MatrixRow::size()) no longer call Rf_isMatrix() + Rf_getAttrib() each time.

Benchmarks

R 4.6.1, arm64 macOS; the same functions compiled against master vs this branch. Results are identical between the two.

code master this PR speedup base R
sugar x + y (1e7) 43.9 ms 9.4 ms 4.7x 3.2 ms
sugar x * y + 2 * x - y (1e7) 61.4 ms 20.8 ms 2.9x 20.2 ms
for (i < x.size()) s += x[i] (1e7) 13.9 ms 5.6 ms 2.5x
CharacterVector loop, x[i] == NA_STRING (1e7) 42.4 ms 15.4 ms 2.8x
List loop, Rf_isNull(x[i]) (1e6) 7.4 ms 4.0 ms 1.9x
matrix loop with j < m.ncol() (1e6) 10.9 ms 1.1 ms 9.9x
MatrixRow loop with j < r.size() (1e6) 10.2 ms 0.7 ms 14.6x

Simple sugar expressions like x + y are still slower than with RCPP_NO_BOUNDS_CHECK, since the (never-taken) branch still blocks vectorization. Letting sugar read plain vector operands without a per-element check would close that gap, but that's a larger change to the sugar machinery, so I've left it for a follow-up.

Notes

  • The cached lengths could only go stale if the underlying SEXP's length (or a Matrix's dim attribute) were changed in place behind Rcpp's back, e.g. via SETLENGTH() / R_resizeVector(). Nothing in Rcpp does this, and Matrix::nrow() already had the same caching behavior.
  • The full tinytest suite passes locally with RunAllRcppTests=yes (1624 tests, 0 failures).

@@ -55,7 +66,7 @@ namespace traits{
void check_index(R_xlen_t i) const {
#ifndef RCPP_NO_BOUNDS_CHECK
if (i >= size) {

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.

Maybe the likely/unlikely macro technique used in the kernel helps too in these cases?

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.

I see there are C++ attributes too, but that means C++20.

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.

per-element bounds check and uncached lengths slow down vector access and sugar

2 participants