Skip to content

Move the smarts of fill_buf() to r_vector.hpp #379

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

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Move the smarts of fill_buf() to r_vector.hpp #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 10 additions & 4 deletions inst/include/cpp11/doubles.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,16 @@ inline typename r_vector<double>::underlying_type* r_vector<double>::get_p(bool
}

template <>
inline void r_vector<double>::const_iterator::fill_buf(R_xlen_t pos) {
length_ = std::min(64_xl, data_->size() - pos);
REAL_GET_REGION(data_->data_, pos, length_, buf_.data());
block_start_ = pos;
inline void r_vector<double>::get_region(
SEXP x, R_xlen_t i, R_xlen_t n,
typename traits::get_underlying_type<double>::type* buf) {
// NOPROTECT: likely too costly to unwind protect here
REAL_GET_REGION(x, i, n, buf);
};

template <>
inline bool r_vector<double>::const_iterator::use_buf(bool is_altrep) {
return is_altrep;
}

typedef r_vector<double> doubles;
Expand Down
14 changes: 10 additions & 4 deletions inst/include/cpp11/integers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ inline typename r_vector<int>::underlying_type* r_vector<int>::get_p(bool is_alt
}

template <>
inline void r_vector<int>::const_iterator::fill_buf(R_xlen_t pos) {
length_ = std::min(64_xl, data_->size() - pos);
INTEGER_GET_REGION(data_->data_, pos, length_, buf_.data());
block_start_ = pos;
inline void r_vector<int>::get_region(
SEXP x, R_xlen_t i, R_xlen_t n,
typename traits::get_underlying_type<int>::type* buf) {
// NOPROTECT: likely too costly to unwind protect here
INTEGER_GET_REGION(x, i, n, buf);
};

template <>
inline bool r_vector<int>::const_iterator::use_buf(bool is_altrep) {
return is_altrep;
}

typedef r_vector<int> integers;
Expand Down
11 changes: 9 additions & 2 deletions inst/include/cpp11/list.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,15 @@ inline typename r_vector<SEXP>::underlying_type* r_vector<SEXP>::get_p(bool, SEX
}

template <>
inline void r_vector<SEXP>::const_iterator::fill_buf(R_xlen_t) {
return;
inline void r_vector<SEXP>::get_region(
SEXP x, R_xlen_t i, R_xlen_t n,
typename traits::get_underlying_type<SEXP>::type* buf) {
cpp11::stop("Unreachable!");
};

template <>
inline bool r_vector<SEXP>::const_iterator::use_buf(bool is_altrep) {
return false;
}

template <>
Expand Down
14 changes: 10 additions & 4 deletions inst/include/cpp11/logicals.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,16 @@ inline typename r_vector<r_bool>::underlying_type* r_vector<r_bool>::get_p(bool
}

template <>
inline void r_vector<r_bool>::const_iterator::fill_buf(R_xlen_t pos) {
length_ = std::min(64_xl, data_->size() - pos);
LOGICAL_GET_REGION(data_->data_, pos, length_, buf_.data());
block_start_ = pos;
inline void r_vector<r_bool>::get_region(
SEXP x, R_xlen_t i, R_xlen_t n,
typename traits::get_underlying_type<r_bool>::type* buf) {
// NOPROTECT: likely too costly to unwind protect here
LOGICAL_GET_REGION(x, i, n, buf);
};

template <>
inline bool r_vector<r_bool>::const_iterator::use_buf(bool is_altrep) {
return is_altrep;
}

typedef r_vector<r_bool> logicals;
Expand Down
28 changes: 20 additions & 8 deletions inst/include/cpp11/r_vector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,16 @@ class r_vector {

private:
/// Implemented in specialization
static bool use_buf(bool is_altrep);
void fill_buf(R_xlen_t pos);
};

private:
/// Implemented in specialization
static underlying_type* get_p(bool is_altrep, SEXP data);
/// Implemented in specialization
static void get_region(SEXP x, R_xlen_t i, R_xlen_t n, underlying_type* buf);
/// Implemented in specialization
static SEXP valid_type(SEXP data);

friend class writable::r_vector<T>;
Expand Down Expand Up @@ -283,6 +286,7 @@ class r_vector : public cpp11::r_vector<T> {
using cpp11::r_vector<T>::const_iterator::pos_;
using cpp11::r_vector<T>::const_iterator::buf_;
using cpp11::r_vector<T>::const_iterator::length_;
using cpp11::r_vector<T>::const_iterator::use_buf;
using cpp11::r_vector<T>::const_iterator::fill_buf;

public:
Expand Down Expand Up @@ -572,15 +576,15 @@ inline typename r_vector<T>::const_iterator r_vector<T>::cend() const {
template <typename T>
r_vector<T>::const_iterator::const_iterator(const r_vector* data, R_xlen_t pos)
: data_(data), pos_(pos), buf_() {
if (data_->is_altrep()) {
if (use_buf(data_->is_altrep())) {
fill_buf(pos);
}
}

template <typename T>
inline typename r_vector<T>::const_iterator& r_vector<T>::const_iterator::operator++() {
++pos_;
if (data_->is_altrep() && pos_ >= block_start_ + length_) {
if (use_buf(data_->is_altrep()) && pos_ >= block_start_ + length_) {
fill_buf(pos_);
}
return *this;
Expand All @@ -589,7 +593,7 @@ inline typename r_vector<T>::const_iterator& r_vector<T>::const_iterator::operat
template <typename T>
inline typename r_vector<T>::const_iterator& r_vector<T>::const_iterator::operator--() {
--pos_;
if (data_->is_altrep() && pos_ > 0 && pos_ < block_start_) {
if (use_buf(data_->is_altrep()) && pos_ > 0 && pos_ < block_start_) {
fill_buf(std::max(0_xl, pos_ - 64));
}
return *this;
Expand All @@ -599,7 +603,7 @@ template <typename T>
inline typename r_vector<T>::const_iterator& r_vector<T>::const_iterator::operator+=(
R_xlen_t i) {
pos_ += i;
if (data_->is_altrep() && pos_ >= block_start_ + length_) {
if (use_buf(data_->is_altrep()) && pos_ >= block_start_ + length_) {
fill_buf(pos_);
}
return *this;
Expand All @@ -609,7 +613,7 @@ template <typename T>
inline typename r_vector<T>::const_iterator& r_vector<T>::const_iterator::operator-=(
R_xlen_t i) {
pos_ -= i;
if (data_->is_altrep() && pos_ >= block_start_ + length_) {
if (use_buf(data_->is_altrep()) && pos_ >= block_start_ + length_) {
fill_buf(std::max(0_xl, pos_ - 64));
}
return *this;
Expand Down Expand Up @@ -666,6 +670,14 @@ inline T r_vector<T>::const_iterator::operator*() const {
}
}

template <typename T>
inline void r_vector<T>::const_iterator::fill_buf(R_xlen_t pos) {
using namespace cpp11::literals;
length_ = std::min(64_xl, data_->size() - pos);
get_region(data_->data_, pos, length_, buf_.data());
block_start_ = pos;
}

namespace writable {

template <typename T>
Expand Down Expand Up @@ -1062,15 +1074,15 @@ r_vector<T>::iterator::iterator(const r_vector& data, R_xlen_t pos)
template <typename T>
inline typename r_vector<T>::iterator& r_vector<T>::iterator::operator++() {
++pos_;
if (data_.is_altrep() && pos_ >= block_start_ + length_) {
if (use_buf(data_.is_altrep()) && pos_ >= block_start_ + length_) {
fill_buf(pos_);
}
return *this;
}

template <typename T>
inline typename r_vector<T>::proxy r_vector<T>::iterator::operator*() const {
if (data_.is_altrep()) {
if (use_buf(data_.is_altrep())) {
return proxy(
data_.data(), pos_,
const_cast<typename r_vector::underlying_type*>(&buf_[pos_ - block_start_]),
Expand All @@ -1084,7 +1096,7 @@ inline typename r_vector<T>::proxy r_vector<T>::iterator::operator*() const {
template <typename T>
inline typename r_vector<T>::iterator& r_vector<T>::iterator::operator+=(R_xlen_t rhs) {
pos_ += rhs;
if (data_.is_altrep() && pos_ >= block_start_ + length_) {
if (use_buf(data_.is_altrep()) && pos_ >= block_start_ + length_) {
fill_buf(pos_);
}
return *this;
Expand Down
15 changes: 10 additions & 5 deletions inst/include/cpp11/raws.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,16 @@ inline typename r_vector<uint8_t>::underlying_type* r_vector<uint8_t>::get_p(
}

template <>
inline void r_vector<uint8_t>::const_iterator::fill_buf(R_xlen_t pos) {
using namespace cpp11::literals;
length_ = std::min(64_xl, data_->size() - pos);
unwind_protect([&] { RAW_GET_REGION(data_->data_, pos, length_, buf_.data()); });
block_start_ = pos;
inline void r_vector<uint8_t>::get_region(
SEXP x, R_xlen_t i, R_xlen_t n,
typename traits::get_underlying_type<uint8_t>::type* buf) {
// NOPROTECT: likely too costly to unwind protect here
RAW_GET_REGION(x, i, n, buf);
Comment on lines -58 to +59
Copy link
Member Author

Choose a reason for hiding this comment

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

No other implementation was using unwind_protect(), so I'm no longer doing it here either

};

template <>
inline bool r_vector<uint8_t>::const_iterator::use_buf(bool is_altrep) {
return is_altrep;
}

typedef r_vector<uint8_t> raws;
Expand Down
11 changes: 9 additions & 2 deletions inst/include/cpp11/strings.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,15 @@ inline typename r_vector<r_string>::underlying_type* r_vector<r_string>::get_p(b
}

template <>
inline void r_vector<r_string>::const_iterator::fill_buf(R_xlen_t) {
return;
inline void r_vector<r_string>::get_region(
SEXP x, R_xlen_t i, R_xlen_t n,
typename traits::get_underlying_type<r_string>::type* buf) {
cpp11::stop("Unreachable!");
};

template <>
inline bool r_vector<r_string>::const_iterator::use_buf(bool is_altrep) {
return false;
}

template <>
Expand Down
Loading