diff --git a/NEWS.md b/NEWS.md index 2c486c35..fcb31e1b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # cpp11 (development version) +* Repeated assignment to a `cpp11::writable::strings` vector through either + `x[i] = elt` or `x.push_back(elt)` is now more performant, at the tradeoff + of slightly less safety (as long as `elt` is actually a `CHARSXP` and `i` is + within bounds, there is no chance of failure, which are the same kind of + invariants placed on the other vector types) (#378). + * Read only `r_vector`s now have a move constructor and move assignment operator (#365). diff --git a/cpp11test/R/cpp11.R b/cpp11test/R/cpp11.R index bb3153e3..038e7b76 100644 --- a/cpp11test/R/cpp11.R +++ b/cpp11test/R/cpp11.R @@ -160,6 +160,14 @@ cpp11_safe_ <- function(x_sxp) { .Call(`_cpp11test_cpp11_safe_`, x_sxp) } +string_proxy_assignment_ <- function() { + .Call(`_cpp11test_string_proxy_assignment_`) +} + +string_push_back_ <- function() { + .Call(`_cpp11test_string_push_back_`) +} + sum_dbl_for_ <- function(x) { .Call(`_cpp11test_sum_dbl_for_`, x) } diff --git a/cpp11test/src/cpp11.cpp b/cpp11test/src/cpp11.cpp index 7cfa36c3..421de637 100644 --- a/cpp11test/src/cpp11.cpp +++ b/cpp11test/src/cpp11.cpp @@ -310,6 +310,20 @@ extern "C" SEXP _cpp11test_cpp11_safe_(SEXP x_sxp) { return cpp11::as_sexp(cpp11_safe_(cpp11::as_cpp>(x_sxp))); END_CPP11 } +// strings.cpp +cpp11::writable::strings string_proxy_assignment_(); +extern "C" SEXP _cpp11test_string_proxy_assignment_() { + BEGIN_CPP11 + return cpp11::as_sexp(string_proxy_assignment_()); + END_CPP11 +} +// strings.cpp +cpp11::writable::strings string_push_back_(); +extern "C" SEXP _cpp11test_string_push_back_() { + BEGIN_CPP11 + return cpp11::as_sexp(string_push_back_()); + END_CPP11 +} // sum.cpp double sum_dbl_for_(cpp11::doubles x); extern "C" SEXP _cpp11test_sum_dbl_for_(SEXP x) { @@ -504,6 +518,8 @@ static const R_CallMethodDef CallEntries[] = { {"_cpp11test_rcpp_sum_int_for_", (DL_FUNC) &_cpp11test_rcpp_sum_int_for_, 1}, {"_cpp11test_remove_altrep", (DL_FUNC) &_cpp11test_remove_altrep, 1}, {"_cpp11test_row_sums", (DL_FUNC) &_cpp11test_row_sums, 1}, + {"_cpp11test_string_proxy_assignment_", (DL_FUNC) &_cpp11test_string_proxy_assignment_, 0}, + {"_cpp11test_string_push_back_", (DL_FUNC) &_cpp11test_string_push_back_, 0}, {"_cpp11test_sum_dbl_accumulate2_", (DL_FUNC) &_cpp11test_sum_dbl_accumulate2_, 1}, {"_cpp11test_sum_dbl_accumulate_", (DL_FUNC) &_cpp11test_sum_dbl_accumulate_, 1}, {"_cpp11test_sum_dbl_for2_", (DL_FUNC) &_cpp11test_sum_dbl_for2_, 1}, diff --git a/cpp11test/src/strings.cpp b/cpp11test/src/strings.cpp new file mode 100644 index 00000000..1db7736b --- /dev/null +++ b/cpp11test/src/strings.cpp @@ -0,0 +1,35 @@ +#include "cpp11/strings.hpp" + +// Test benchmark for string proxy assignment performance. +// We don't unwind_protect() before each `SET_STRING_ELT()` call, +// as that kills performance. +[[cpp11::register]] cpp11::writable::strings string_proxy_assignment_() { + R_xlen_t size = 100000; + + cpp11::writable::strings x(size); + + cpp11::r_string elt(NA_STRING); + + for (R_xlen_t i = 0; i < size; ++i) { + x[i] = elt; + } + + return x; +} + +// Test benchmark for string push back performance. +// We don't unwind_protect() before each `SET_STRING_ELT()` call, +// as that kills performance. +[[cpp11::register]] cpp11::writable::strings string_push_back_() { + R_xlen_t size = 100000; + + cpp11::writable::strings x; + + cpp11::r_string elt(NA_STRING); + + for (R_xlen_t i = 0; i < size; ++i) { + x.push_back(elt); + } + + return x; +} diff --git a/inst/include/cpp11/strings.hpp b/inst/include/cpp11/strings.hpp index 64cee045..3a574e08 100644 --- a/inst/include/cpp11/strings.hpp +++ b/inst/include/cpp11/strings.hpp @@ -67,7 +67,8 @@ inline void r_vector::set_elt( template <> inline typename r_vector::proxy& r_vector::proxy::operator=( const r_string& rhs) { - unwind_protect([&] { SET_STRING_ELT(data_, index_, rhs); }); + // NOPROTECT: likely too costly to unwind protect every elt + SET_STRING_ELT(data_, index_, rhs); return *this; } @@ -169,7 +170,8 @@ inline void r_vector::push_back(r_string value) { while (length_ >= capacity_) { reserve(capacity_ == 0 ? 1 : capacity_ *= 2); } - unwind_protect([&] { SET_STRING_ELT(data_, length_, value); }); + // NOPROTECT: likely too costly to unwind protect every elt + SET_STRING_ELT(data_, length_, value); ++length_; }