From de977ea15c39e9a42b6d09d27c06337d30f77ccd Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Wed, 30 Sep 2026 09:54:31 -0700 Subject: [PATCH 1/3] speed up bounds-checked element access and cache vector lengths Closes #1510. --- ChangeLog | 9 +++++++++ inst/include/Rcpp/vector/Matrix.h | 30 +++++++++++++++------------- inst/include/Rcpp/vector/SubMatrix.h | 3 ++- inst/include/Rcpp/vector/Vector.h | 6 +++--- inst/include/Rcpp/vector/traits.h | 24 +++++++++++++++++----- 5 files changed, 49 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7ec6570b6..366cb99c8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2026-09-30 Kevin Ushey + + * inst/include/Rcpp/vector/traits.h: Move the out-of-bounds warning into + a cold, out-of-line helper; cache the vector length in proxy_cache + * inst/include/Rcpp/vector/Vector.h: Return the cached length from + size() and length() rather than calling Rf_xlength() each time + * inst/include/Rcpp/vector/Matrix.h: Cache the number of columns + * inst/include/Rcpp/vector/SubMatrix.h: Idem + 2026-09-22 Iñaki Ucar * inst/include/Rcpp/sugar/matrix/col.h: Fix Col constructor using ncol() diff --git a/inst/include/Rcpp/vector/Matrix.h b/inst/include/Rcpp/vector/Matrix.h index 137de2500..e237f7ce9 100644 --- a/inst/include/Rcpp/vector/Matrix.h +++ b/inst/include/Rcpp/vector/Matrix.h @@ -28,6 +28,7 @@ namespace Rcpp{ template class StoragePolicy = PreserveStorage > class Matrix : public Vector, public MatrixBase > { int nrows ; + int ncols ; public: using Vector::size; // disambiguate diamond pattern for g++-6 and later @@ -49,33 +50,33 @@ class Matrix : public Vector, public MatrixBase - Matrix( const int& nrows_, const int& ncols, Iterator start ) : - VECTOR( start, start + (static_cast(nrows_)*ncols) ), - nrows(nrows_) + Matrix( const int& nrows_, const int& ncols_, Iterator start ) : + VECTOR( start, start + (static_cast(nrows_)*ncols_) ), + nrows(nrows_), ncols(ncols_) { VECTOR::attr( "dim" ) = Dimension( nrows, ncols ) ; } - Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n) {} + Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n), ncols(n) {} - Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows) {} + Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows), ncols(other.ncols) {} template - Matrix( const MatrixBase& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast(other.nrow()), static_cast(other.ncol()) ) ), nrows(static_cast(other.nrow())) { + Matrix( const MatrixBase& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast(other.nrow()), static_cast(other.ncol()) ) ), nrows(static_cast(other.nrow())), ncols(static_cast(other.ncol())) { import_matrix_expression( other, nrows, ncol() ) ; } @@ -86,20 +87,21 @@ class Matrix : public Vector, public MatrixBase& ) ; - explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()) {} + explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()), ncols(obj.ncol()) {} inline int ncol() const { - return VECTOR::dims()[1]; + return ncols ; } inline int nrow() const { return nrows ; } inline int cols() const { - return VECTOR::dims()[1]; + return ncols ; } inline int rows() const { return nrows ; diff --git a/inst/include/Rcpp/vector/SubMatrix.h b/inst/include/Rcpp/vector/SubMatrix.h index 142b6591e..79fb4b2c4 100644 --- a/inst/include/Rcpp/vector/SubMatrix.h +++ b/inst/include/Rcpp/vector/SubMatrix.h @@ -56,7 +56,7 @@ class SubMatrix : public Rcpp::MatrixBase< RTYPE, true, SubMatrix > { } ; template class StoragePolicy > -Matrix::Matrix( const SubMatrix& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()) { +Matrix::Matrix( const SubMatrix& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()), ncols(sub.ncol()) { int nc = sub.ncol() ; iterator start = VECTOR::begin() ; iterator rhs_it ; @@ -73,6 +73,7 @@ Matrix& Matrix::operator=( const SubMa int nc = sub.ncol(), nr = sub.nrow() ; if( nc != nrow() || nr != ncol() ){ nrows = nr ; + ncols = nc ; VECTOR::set__( Rf_allocMatrix( RTYPE, nr, nc ) ) ; } iterator start = VECTOR::begin() ; diff --git a/inst/include/Rcpp/vector/Vector.h b/inst/include/Rcpp/vector/Vector.h index bfb78106c..8abd8c221 100644 --- a/inst/include/Rcpp/vector/Vector.h +++ b/inst/include/Rcpp/vector/Vector.h @@ -263,17 +263,17 @@ class Vector : #endif /** - * the length of the vector, uses Rf_xlength + * the length of the vector, as cached by update() */ inline R_xlen_t length() const { - return ::Rf_xlength( Storage::get__() ) ; + return cache.get_size() ; } /** * alias of length */ inline R_xlen_t size() const { - return ::Rf_xlength( Storage::get__() ) ; + return cache.get_size() ; } /** diff --git a/inst/include/Rcpp/vector/traits.h b/inst/include/Rcpp/vector/traits.h index 3e1dcecec..9c8ddbfe5 100644 --- a/inst/include/Rcpp/vector/traits.h +++ b/inst/include/Rcpp/vector/traits.h @@ -24,6 +24,16 @@ namespace Rcpp{ namespace traits{ + // Kept out of line and marked cold so that the (almost never taken) + // warning path doesn't bloat element access and block optimization of + // hot loops, e.g. in sugar expressions. +#if defined(__GNUC__) + __attribute__((noinline, cold)) +#endif + inline void warn_index_out_of_bounds(R_xlen_t i, R_xlen_t size) { + warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov + } + template class StoragePolicy = PreserveStorage > class r_vector_cache{ public: @@ -38,11 +48,12 @@ namespace traits{ inline void update( const VECTOR& v ) { start = ::Rcpp::internal::r_vector_start(v) ; - size = v.size(); + size = ::Rf_xlength(v.get__()); } inline iterator get() const { return start; } inline const_iterator get_const() const { return start; } + inline R_xlen_t get_size() const { return size; } inline proxy ref() { check_index(0); return start[0] ;} inline proxy ref(R_xlen_t i) { check_index(i); return start[i] ; } @@ -55,7 +66,7 @@ namespace traits{ void check_index(R_xlen_t i) const { #ifndef RCPP_NO_BOUNDS_CHECK if (i >= size) { - warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov + warn_index_out_of_bounds(i, size); // #nocov } #endif } @@ -73,13 +84,15 @@ namespace traits{ typedef typename r_vector_proxy::type proxy ; typedef typename r_vector_const_proxy::type const_proxy ; - proxy_cache(): p(0){} + proxy_cache(): p(0), size(0){} ~proxy_cache(){} void update( const VECTOR& v ){ p = const_cast(&v) ; + size = ::Rf_xlength(v.get__()); } inline iterator get() const { return iterator( proxy(*p, 0 ) ) ;} inline const_iterator get_const() const { return const_iterator( const_proxy(*p, 0) ) ; } + inline R_xlen_t get_size() const { return size; } inline proxy ref() { check_index(0); return proxy(*p,0) ; } inline proxy ref(R_xlen_t i) { check_index(i); return proxy(*p,i);} @@ -89,11 +102,12 @@ namespace traits{ private: VECTOR* p ; + R_xlen_t size ; void check_index(R_xlen_t i) const { #ifndef RCPP_NO_BOUNDS_CHECK - if (i >= p->size()) { - warning("subscript out of bounds (index %s >= vector size %s)", i, p->size()); // #nocov + if (i >= size) { + warn_index_out_of_bounds(i, size); // #nocov } #endif } From 79464429150d956e8b1d446b1b92411d07e76d00 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Wed, 30 Sep 2026 09:55:02 -0700 Subject: [PATCH 2/3] update NEWS --- inst/NEWS.Rd | 3 +++ 1 file changed, 3 insertions(+) diff --git a/inst/NEWS.Rd b/inst/NEWS.Rd index 0bac0eee9..de5b6e0b7 100644 --- a/inst/NEWS.Rd +++ b/inst/NEWS.Rd @@ -27,6 +27,9 @@ warnings (Iñaki in \ghpr{1508} closing \ghit{1497}) \item The \code{col} constructor now uses \code{nrow} in an initialization (Iñaki in \ghpr{1509}) + \item Bounds-checked element access is faster, and vector lengths and + matrix column counts are now cached (Kevin in \ghpr{1511} closing + \ghit{1510}) } \item Changes in Rcpp Documentation: \itemize{ From a447f303db5278b604c52f68758cf822a0e16128 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Fri, 9 Oct 2026 10:09:37 -0700 Subject: [PATCH 3/3] keep vector and matrix layouts unchanged Caching the length in proxy_cache and the column count in Matrix added data members, changing the layout of List, CharacterVector and Matrix. Packages pass these by reference across shared libraries compiled against different Rcpp versions: rust hands a List to revdbayes through an XPtr function pointer, and lite, threshr and evmissing reach the same path. In the 2026-10-06 reverse-dependency run revdbayes read a size that rust never wrote and failed name lookup with 'Index out of bounds'. ncol() is now derived from the cached length and nrow(), and proxy_cache reads the length on demand; only the cold warning helper and the cached atomic-vector length remain. --- ChangeLog | 12 +++++---- inst/NEWS.Rd | 7 ++++-- inst/include/Rcpp/vector/Matrix.h | 37 ++++++++++++++++------------ inst/include/Rcpp/vector/SubMatrix.h | 3 +-- inst/include/Rcpp/vector/traits.h | 10 +++++--- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/ChangeLog b/ChangeLog index 366cb99c8..bf4308ef2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,11 +1,13 @@ 2026-09-30 Kevin Ushey * inst/include/Rcpp/vector/traits.h: Move the out-of-bounds warning into - a cold, out-of-line helper; cache the vector length in proxy_cache - * inst/include/Rcpp/vector/Vector.h: Return the cached length from - size() and length() rather than calling Rf_xlength() each time - * inst/include/Rcpp/vector/Matrix.h: Cache the number of columns - * inst/include/Rcpp/vector/SubMatrix.h: Idem + a cold, out-of-line helper; add get_size() to the vector caches + * inst/include/Rcpp/vector/Vector.h: Return the length cached by + r_vector_cache from size() and length() rather than calling + Rf_xlength() each time + * inst/include/Rcpp/vector/Matrix.h: Derive ncol() from the cached + length and nrow() instead of reading the dim attribute; document that + no data members may be added, as the layout is shared across packages 2026-09-22 Iñaki Ucar diff --git a/inst/NEWS.Rd b/inst/NEWS.Rd index de5b6e0b7..d70841ee7 100644 --- a/inst/NEWS.Rd +++ b/inst/NEWS.Rd @@ -27,8 +27,11 @@ warnings (Iñaki in \ghpr{1508} closing \ghit{1497}) \item The \code{col} constructor now uses \code{nrow} in an initialization (Iñaki in \ghpr{1509}) - \item Bounds-checked element access is faster, and vector lengths and - matrix column counts are now cached (Kevin in \ghpr{1511} closing + \item Bounds-checked element access is faster, \code{size()} of + atomic vectors uses the cached length, and \code{ncol()} no longer + reads the \code{dim} attribute; object layouts are unchanged, as + packages such as \pkg{rust} and \pkg{revdbayes} exchange Rcpp + objects across shared libraries (Kevin in \ghpr{1511} closing \ghit{1510}) } \item Changes in Rcpp Documentation: diff --git a/inst/include/Rcpp/vector/Matrix.h b/inst/include/Rcpp/vector/Matrix.h index e237f7ce9..8bef4fe4b 100644 --- a/inst/include/Rcpp/vector/Matrix.h +++ b/inst/include/Rcpp/vector/Matrix.h @@ -27,8 +27,11 @@ namespace Rcpp{ template class StoragePolicy = PreserveStorage > class Matrix : public Vector, public MatrixBase > { + // The only data member. Packages pass Rcpp objects by reference across + // shared-library boundaries (e.g. rust calls into revdbayes through XPtr + // function pointers), and the two sides may have been compiled against + // different Rcpp versions. Adding a member changes the layout they share. int nrows ; - int ncols ; public: using Vector::size; // disambiguate diamond pattern for g++-6 and later @@ -50,33 +53,33 @@ class Matrix : public Vector, public MatrixBase - Matrix( const int& nrows_, const int& ncols_, Iterator start ) : - VECTOR( start, start + (static_cast(nrows_)*ncols_) ), - nrows(nrows_), ncols(ncols_) + Matrix( const int& nrows_, const int& ncols, Iterator start ) : + VECTOR( start, start + (static_cast(nrows_)*ncols) ), + nrows(nrows_) { VECTOR::attr( "dim" ) = Dimension( nrows, ncols ) ; } - Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n), ncols(n) {} + Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n) {} - Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows), ncols(other.ncols) {} + Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows) {} template - Matrix( const MatrixBase& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast(other.nrow()), static_cast(other.ncol()) ) ), nrows(static_cast(other.nrow())), ncols(static_cast(other.ncol())) { + Matrix( const MatrixBase& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast(other.nrow()), static_cast(other.ncol()) ) ), nrows(static_cast(other.nrow())) { import_matrix_expression( other, nrows, ncol() ) ; } @@ -87,21 +90,23 @@ class Matrix : public Vector, public MatrixBase& ) ; - explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()), ncols(obj.ncol()) {} + explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()) {} + // A matrix has exactly nrow * ncol elements, so the column count follows + // from the (cached) length without reading the dim attribute. Only an + // empty matrix, which may still have columns, needs the attribute. inline int ncol() const { - return ncols ; + return nrows == 0 ? VECTOR::dims()[1] : static_cast( VECTOR::size() / nrows ) ; } inline int nrow() const { return nrows ; } inline int cols() const { - return ncols ; + return ncol() ; } inline int rows() const { return nrows ; diff --git a/inst/include/Rcpp/vector/SubMatrix.h b/inst/include/Rcpp/vector/SubMatrix.h index 79fb4b2c4..142b6591e 100644 --- a/inst/include/Rcpp/vector/SubMatrix.h +++ b/inst/include/Rcpp/vector/SubMatrix.h @@ -56,7 +56,7 @@ class SubMatrix : public Rcpp::MatrixBase< RTYPE, true, SubMatrix > { } ; template class StoragePolicy > -Matrix::Matrix( const SubMatrix& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()), ncols(sub.ncol()) { +Matrix::Matrix( const SubMatrix& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()) { int nc = sub.ncol() ; iterator start = VECTOR::begin() ; iterator rhs_it ; @@ -73,7 +73,6 @@ Matrix& Matrix::operator=( const SubMa int nc = sub.ncol(), nr = sub.nrow() ; if( nc != nrow() || nr != ncol() ){ nrows = nr ; - ncols = nc ; VECTOR::set__( Rf_allocMatrix( RTYPE, nr, nc ) ) ; } iterator start = VECTOR::begin() ; diff --git a/inst/include/Rcpp/vector/traits.h b/inst/include/Rcpp/vector/traits.h index 9c8ddbfe5..9a3086e39 100644 --- a/inst/include/Rcpp/vector/traits.h +++ b/inst/include/Rcpp/vector/traits.h @@ -84,15 +84,17 @@ namespace traits{ typedef typename r_vector_proxy::type proxy ; typedef typename r_vector_const_proxy::type const_proxy ; - proxy_cache(): p(0), size(0){} + proxy_cache(): p(0){} ~proxy_cache(){} void update( const VECTOR& v ){ p = const_cast(&v) ; - size = ::Rf_xlength(v.get__()); } inline iterator get() const { return iterator( proxy(*p, 0 ) ) ;} inline const_iterator get_const() const { return const_iterator( const_proxy(*p, 0) ) ; } - inline R_xlen_t get_size() const { return size; } + // Not cached: a size member here would change the layout of List and + // CharacterVector, which packages pass by reference across + // shared-library boundaries (see the note in Matrix.h). + inline R_xlen_t get_size() const { return ::Rf_xlength(p->get__()); } inline proxy ref() { check_index(0); return proxy(*p,0) ; } inline proxy ref(R_xlen_t i) { check_index(i); return proxy(*p,i);} @@ -102,10 +104,10 @@ namespace traits{ private: VECTOR* p ; - R_xlen_t size ; void check_index(R_xlen_t i) const { #ifndef RCPP_NO_BOUNDS_CHECK + R_xlen_t size = get_size() ; if (i >= size) { warn_index_out_of_bounds(i, size); // #nocov }