From de977ea15c39e9a42b6d09d27c06337d30f77ccd Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Wed, 30 Sep 2026 09:54:31 -0700 Subject: [PATCH 1/2] 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/2] 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{