Skip to content

Copy constructor for writable vectors is duplicating the SEXP too many times #369

Closed
@DavisVaughan

Description

@DavisVaughan

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:

inline r_vector<T>::r_vector(const r_vector<T>& rhs)
: cpp11::r_vector<T>(safe[Rf_shallow_duplicate](rhs)),
protect_(detail::store::insert(data_)),
capacity_(rhs.capacity_) {}

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   

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions