Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kevinushey wants to merge 3 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de977ea
speed up bounds-checked element access and cache vector lengths
kevinushey Sep 30, 2026
7946442
update NEWS
kevinushey Sep 30, 2026
a447f30
keep vector and matrix layouts unchanged
kevinushey Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
2026-09-30 Kevin Ushey <kevinushey@gmail.com>

* inst/include/Rcpp/vector/traits.h: Move the out-of-bounds warning into
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 <iucar@fedoraproject.org>

* inst/include/Rcpp/sugar/matrix/col.h: Fix Col constructor using ncol()
Expand Down
6 changes: 6 additions & 0 deletions inst/NEWS.Rd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
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, \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:
\itemize{
Expand Down
11 changes: 9 additions & 2 deletions inst/include/Rcpp/vector/Matrix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ namespace Rcpp{

template <int RTYPE, template <class> class StoragePolicy = PreserveStorage >
class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, true, Matrix<RTYPE,StoragePolicy> > {
// 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 ;

public:
Expand Down Expand Up @@ -92,14 +96,17 @@ class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, tru

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 VECTOR::dims()[1];
return nrows == 0 ? VECTOR::dims()[1] : static_cast<int>( VECTOR::size() / nrows ) ;
}
inline int nrow() const {
return nrows ;
}
inline int cols() const {
return VECTOR::dims()[1];
return ncol() ;
}
inline int rows() const {
return nrows ;
Expand Down
6 changes: 3 additions & 3 deletions inst/include/Rcpp/vector/Vector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() ;
}

/**
Expand Down
24 changes: 20 additions & 4 deletions inst/include/Rcpp/vector/traits.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <int RTYPE, template <class> class StoragePolicy = PreserveStorage >
class r_vector_cache{
public:
Expand All @@ -38,11 +48,12 @@ namespace traits{

inline void update( const VECTOR& v ) {
start = ::Rcpp::internal::r_vector_start<RTYPE>(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] ; }
Expand All @@ -55,7 +66,7 @@ namespace traits{
void check_index(R_xlen_t i) const {
#ifndef RCPP_NO_BOUNDS_CHECK
if (i >= size) {

@Enchufa2 Enchufa2 Sep 30, 2026

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?

@Enchufa2 Enchufa2 Sep 30, 2026

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.

@kevinushey kevinushey Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good thought -- I checked, and it turns out __builtin_expect would be redundant with what's already here. Both GCC and Clang treat any branch leading to a call of a cold function as unlikely, so the __attribute__((noinline, cold)) on warn_index_out_of_bounds() already provides the same hint as if (unlikely(i >= size)).

To confirm, I compiled a few hot loops (a reduction, a fill, and a sugar expression over NumericVector) with this branch vs. this branch plus __builtin_expect(!!(i >= size), 0) in check_index(), using Apple clang 21 and gcc 16 at -O2:

  • gcc: byte-for-byte identical output apart from swapped compare operands. gcc also drops the bounds check entirely from the reduction loop here, since the loop bound and the checked size are now the same cached value.
  • clang: the only difference is which way the loop exit block falls through; the instruction count differs by one and the sugar function is identical.
  • Timings on a 1e6-element vector were within noise across the two variants for both compilers.

Interestingly, keeping __builtin_expect but removing cold does change gcc's output: with cold, gcc keeps hot-loop values in caller-saved registers and spills around the (never-taken) warning call; without it, it reserves callee-saved registers in the prologue instead. So cold is doing slightly more than expect alone would.

Portability would be the same either way, since __builtin_expect needs the same __GNUC__ guard, and MSVC has neither (the portable spelling is C++20's [[unlikely]], which we can't require). So I'll leave it as is.

warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov
warn_index_out_of_bounds(i, size); // #nocov
}
#endif
}
Expand All @@ -80,6 +91,10 @@ namespace traits{
}
inline iterator get() const { return iterator( proxy(*p, 0 ) ) ;}
inline const_iterator get_const() const { return const_iterator( const_proxy(*p, 0) ) ; }
// 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);}
Expand All @@ -92,8 +107,9 @@ namespace traits{

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
R_xlen_t size = get_size() ;
if (i >= size) {
warn_index_out_of_bounds(i, size); // #nocov
}
#endif
}
Expand Down
Loading