Description
As of #362, we always have to do an allocation to truncate before returning a SEXP from a writable we've pushed to with push_back()
. That results in one extra allocation in this copy constructor some of the time:
cpp11/inst/include/cpp11/r_vector.hpp
Lines 795 to 798 in 6189417
It's subtle, but the operator SEXP()
operator is called here before doing the shallow duplicate. So we end up with 2 duplications:
- One from the truncation in the
operator SEXP()
(which previously just set a growable bit / truelength and was free) - One from the
Rf_shallow_duplicate()
The right thing to do here is to take full control over this copy constructor rather than trying to push responsibility up to the read only variant. I think we are going to need to call Rf_xlengthgets()
in here ourselves. And I think the resulting copy should end up with capacity_ = rhs.length_
rather than capacity_ = rhs.capacity_
, i.e. the copy should not assume that it's capacity needs to be any larger than the elements it needs to hold.
#include "cpp11/integers.hpp"
[[cpp11::register]] cpp11::writable::integers test_() {
cpp11::writable::integers x(100000);
// Doubles the capacity to 200000
x.push_back(1);
// Calls writable copy constructor, currently this:
// - Truncates when converting to SEXP through an allocation
// - `Rf_shallow_duplicate()`s that SEXP before passing on to the read only ctor
cpp11::writable::integers y(x);
return y;
}
> profmem::profmem(test_())
Rprofmem memory profiling of:
test_()
Memory allocations:
what bytes calls
1 alloc 400048 test_() -> .Call()
2 alloc 800048 test_() -> .Call()
3 alloc 400056 test_() -> .Call()
4 alloc 400056 test_() -> .Call() # <- this one is extra
total 2000208