Skip to content

Commit de977ea

Browse files
kevinusheykevinushey
committed
speed up bounds-checked element access and cache vector lengths
Closes #1510.
1 parent d7be2c3 commit de977ea

5 files changed

Lines changed: 49 additions & 23 deletions

File tree

‎ChangeLog‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
2026-09-30 Kevin Ushey <kevinushey@gmail.com>
2+
3+
* inst/include/Rcpp/vector/traits.h: Move the out-of-bounds warning into
4+
a cold, out-of-line helper; cache the vector length in proxy_cache
5+
* inst/include/Rcpp/vector/Vector.h: Return the cached length from
6+
size() and length() rather than calling Rf_xlength() each time
7+
* inst/include/Rcpp/vector/Matrix.h: Cache the number of columns
8+
* inst/include/Rcpp/vector/SubMatrix.h: Idem
9+
110
2026-09-22 Iñaki Ucar <iucar@fedoraproject.org>
211

312
* inst/include/Rcpp/sugar/matrix/col.h: Fix Col constructor using ncol()

‎inst/include/Rcpp/vector/Matrix.h‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ namespace Rcpp{
2828
template <int RTYPE, template <class> class StoragePolicy = PreserveStorage >
2929
class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, true, Matrix<RTYPE,StoragePolicy> > {
3030
int nrows ;
31+
int ncols ;
3132

3233
public:
3334
using Vector<RTYPE, StoragePolicy>::size; // disambiguate diamond pattern for g++-6 and later
@@ -49,33 +50,33 @@ class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, tru
4950
typedef typename VECTOR::Proxy Proxy ;
5051
typedef typename VECTOR::const_Proxy const_Proxy ;
5152

52-
Matrix() : VECTOR(Dimension(0, 0)), nrows(0) {}
53+
Matrix() : VECTOR(Dimension(0, 0)), nrows(0), ncols(0) {}
5354

54-
Matrix(SEXP x) : VECTOR(x), nrows( VECTOR::dims()[0] ) {}
55+
Matrix(SEXP x) : VECTOR(x), nrows( VECTOR::dims()[0] ), ncols( VECTOR::dims()[1] ) {}
5556

56-
Matrix( const Dimension& dims) : VECTOR( Rf_allocMatrix( RTYPE, dims[0], dims[1] ) ), nrows(dims[0]) {
57+
Matrix( const Dimension& dims) : VECTOR( Rf_allocMatrix( RTYPE, dims[0], dims[1] ) ), nrows(dims[0]), ncols(dims[1]) {
5758
if( dims.size() != 2 ) throw not_a_matrix();
5859
VECTOR::init() ;
5960
}
60-
Matrix( const int& nrows_, const int& ncols) : VECTOR( Dimension( nrows_, ncols ) ),
61-
nrows(nrows_)
61+
Matrix( const int& nrows_, const int& ncols_) : VECTOR( Dimension( nrows_, ncols_ ) ),
62+
nrows(nrows_), ncols(ncols_)
6263
{}
6364

6465
template <typename Iterator>
65-
Matrix( const int& nrows_, const int& ncols, Iterator start ) :
66-
VECTOR( start, start + (static_cast<R_xlen_t>(nrows_)*ncols) ),
67-
nrows(nrows_)
66+
Matrix( const int& nrows_, const int& ncols_, Iterator start ) :
67+
VECTOR( start, start + (static_cast<R_xlen_t>(nrows_)*ncols_) ),
68+
nrows(nrows_), ncols(ncols_)
6869
{
6970
VECTOR::attr( "dim" ) = Dimension( nrows, ncols ) ;
7071
}
7172

72-
Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n) {}
73+
Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n), ncols(n) {}
7374

7475

75-
Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows) {}
76+
Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows), ncols(other.ncols) {}
7677

7778
template <bool NA, typename MAT>
78-
Matrix( const MatrixBase<RTYPE,NA,MAT>& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast<int>(other.nrow()), static_cast<int>(other.ncol()) ) ), nrows(static_cast<int>(other.nrow())) {
79+
Matrix( const MatrixBase<RTYPE,NA,MAT>& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast<int>(other.nrow()), static_cast<int>(other.ncol()) ) ), nrows(static_cast<int>(other.nrow())), ncols(static_cast<int>(other.ncol())) {
7980
import_matrix_expression<NA,MAT>( other, nrows, ncol() ) ;
8081
}
8182

@@ -86,20 +87,21 @@ class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, tru
8687
if( ! ::Rf_isMatrix(x) ) throw not_a_matrix();
8788
VECTOR::set__( x ) ;
8889
nrows = other.nrows ;
90+
ncols = other.ncols ;
8991
return *this ;
9092
}
9193
Matrix& operator=( const SubMatrix<RTYPE>& ) ;
9294

93-
explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()) {}
95+
explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()), ncols(obj.ncol()) {}
9496

9597
inline int ncol() const {
96-
return VECTOR::dims()[1];
98+
return ncols ;
9799
}
98100
inline int nrow() const {
99101
return nrows ;
100102
}
101103
inline int cols() const {
102-
return VECTOR::dims()[1];
104+
return ncols ;
103105
}
104106
inline int rows() const {
105107
return nrows ;

‎inst/include/Rcpp/vector/SubMatrix.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class SubMatrix : public Rcpp::MatrixBase< RTYPE, true, SubMatrix<RTYPE> > {
5656
} ;
5757

5858
template <int RTYPE, template <class> class StoragePolicy >
59-
Matrix<RTYPE,StoragePolicy>::Matrix( const SubMatrix<RTYPE>& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()) {
59+
Matrix<RTYPE,StoragePolicy>::Matrix( const SubMatrix<RTYPE>& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()), ncols(sub.ncol()) {
6060
int nc = sub.ncol() ;
6161
iterator start = VECTOR::begin() ;
6262
iterator rhs_it ;
@@ -73,6 +73,7 @@ Matrix<RTYPE,StoragePolicy>& Matrix<RTYPE,StoragePolicy>::operator=( const SubMa
7373
int nc = sub.ncol(), nr = sub.nrow() ;
7474
if( nc != nrow() || nr != ncol() ){
7575
nrows = nr ;
76+
ncols = nc ;
7677
VECTOR::set__( Rf_allocMatrix( RTYPE, nr, nc ) ) ;
7778
}
7879
iterator start = VECTOR::begin() ;

‎inst/include/Rcpp/vector/Vector.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,17 +263,17 @@ class Vector :
263263
#endif
264264

265265
/**
266-
* the length of the vector, uses Rf_xlength
266+
* the length of the vector, as cached by update()
267267
*/
268268
inline R_xlen_t length() const {
269-
return ::Rf_xlength( Storage::get__() ) ;
269+
return cache.get_size() ;
270270
}
271271

272272
/**
273273
* alias of length
274274
*/
275275
inline R_xlen_t size() const {
276-
return ::Rf_xlength( Storage::get__() ) ;
276+
return cache.get_size() ;
277277
}
278278

279279
/**

‎inst/include/Rcpp/vector/traits.h‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@
2424
namespace Rcpp{
2525
namespace traits{
2626

27+
// Kept out of line and marked cold so that the (almost never taken)
28+
// warning path doesn't bloat element access and block optimization of
29+
// hot loops, e.g. in sugar expressions.
30+
#if defined(__GNUC__)
31+
__attribute__((noinline, cold))
32+
#endif
33+
inline void warn_index_out_of_bounds(R_xlen_t i, R_xlen_t size) {
34+
warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov
35+
}
36+
2737
template <int RTYPE, template <class> class StoragePolicy = PreserveStorage >
2838
class r_vector_cache{
2939
public:
@@ -38,11 +48,12 @@ namespace traits{
3848

3949
inline void update( const VECTOR& v ) {
4050
start = ::Rcpp::internal::r_vector_start<RTYPE>(v) ;
41-
size = v.size();
51+
size = ::Rf_xlength(v.get__());
4252
}
4353

4454
inline iterator get() const { return start; }
4555
inline const_iterator get_const() const { return start; }
56+
inline R_xlen_t get_size() const { return size; }
4657

4758
inline proxy ref() { check_index(0); return start[0] ;}
4859
inline proxy ref(R_xlen_t i) { check_index(i); return start[i] ; }
@@ -55,7 +66,7 @@ namespace traits{
5566
void check_index(R_xlen_t i) const {
5667
#ifndef RCPP_NO_BOUNDS_CHECK
5768
if (i >= size) {
58-
warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov
69+
warn_index_out_of_bounds(i, size); // #nocov
5970
}
6071
#endif
6172
}
@@ -73,13 +84,15 @@ namespace traits{
7384
typedef typename r_vector_proxy<RTYPE, StoragePolicy>::type proxy ;
7485
typedef typename r_vector_const_proxy<RTYPE, StoragePolicy>::type const_proxy ;
7586

76-
proxy_cache(): p(0){}
87+
proxy_cache(): p(0), size(0){}
7788
~proxy_cache(){}
7889
void update( const VECTOR& v ){
7990
p = const_cast<VECTOR*>(&v) ;
91+
size = ::Rf_xlength(v.get__());
8092
}
8193
inline iterator get() const { return iterator( proxy(*p, 0 ) ) ;}
8294
inline const_iterator get_const() const { return const_iterator( const_proxy(*p, 0) ) ; }
95+
inline R_xlen_t get_size() const { return size; }
8396

8497
inline proxy ref() { check_index(0); return proxy(*p,0) ; }
8598
inline proxy ref(R_xlen_t i) { check_index(i); return proxy(*p,i);}
@@ -89,11 +102,12 @@ namespace traits{
89102

90103
private:
91104
VECTOR* p ;
105+
R_xlen_t size ;
92106

93107
void check_index(R_xlen_t i) const {
94108
#ifndef RCPP_NO_BOUNDS_CHECK
95-
if (i >= p->size()) {
96-
warning("subscript out of bounds (index %s >= vector size %s)", i, p->size()); // #nocov
109+
if (i >= size) {
110+
warn_index_out_of_bounds(i, size); // #nocov
97111
}
98112
#endif
99113
}

0 commit comments

Comments
 (0)