Use the experimental resizable vectors API

Thanks to Luke Tierney for introducing the API and helping with the migration.

Author: aitap

inst/tests/tests.Rraw

@@ -19453,7 +19453,8 @@ test(2290.4, DT[, `:=`(a = 2, c := 3)], error="It looks like you re-used `:=` in
 df = data.frame(a=1:3)
 setDT(df)
 attr(df, "att") = 1
-test(2291.1, set(df, NULL, "new", "new"), error="attributes .* have been reassigned")
+##test(2291.1, set(df, NULL, "new", "new"), error="attributes .* have been reassigned")
+test(2291.1, set(df, NULL, "new", "new"), error="either been loaded from disk.*or constructed manually.*Please run setDT.*setalloccol.*on it first")
 
 # ns-qualified bysub error, #6493
 DT = data.table(a = 1)

src/assign.c

@@ -2,7 +2,6 @@
 
 static void finalizer(SEXP p)
 {
-  SEXP x;
   R_len_t n, l, tl;
   if(!R_ExternalPtrAddr(p)) internal_error(__func__, "didn't receive an ExternalPtr"); // # nocov
   p = R_ExternalPtrTag(p);
@@ -16,9 +15,12 @@ static void finalizer(SEXP p)
     // already, so nothing to do (but almost never the case).
     return;
   }
+#ifdef DODO
+  SEXP x;
   x = PROTECT(allocVector(INTSXP, 50));  // 50 so it's big enough to be on LargeVector heap. See NodeClassSize in memory.c:allocVector
-                                         // INTSXP rather than VECSXP so that GC doesn't inspect contents after LENGTH (thanks to Karl Miller, Jul 2015)
+                                         // INTSXP rather than VECSXP so that GC doesn't inspect contents after LENGTH (thanks to Karl Millar, Jul 2015)
   SETLENGTH(x,50+n*2*sizeof(void *)/4);  // 1*n for the names, 1*n for the VECSXP itself (both are over allocated).
+#endif
   UNPROTECT(1);
   return;
 }
@@ -69,7 +71,7 @@ happens too late after GC has already released the memory, and ii) copies by bas
 [<- in write.table just before test 894) allocate at length LENGTH but copy the TRUELENGTH over.
 If the finalizer sets LENGTH to TRUELENGTH, that's a fail as it wasn't really allocated at
 TRUELENGTH when R did the copy.
-Karl Miller suggested an ENVSXP so that restoring LENGTH in finalizer should work. This is the
+Karl Millar suggested an ENVSXP so that restoring LENGTH in finalizer should work. This is the
 closest I got to getting it to pass all tests :
 
   SEXP env = PROTECT(allocSExp(ENVSXP));
@@ -151,7 +153,7 @@ static SEXP shallow(SEXP dt, SEXP cols, R_len_t n)
   // called from alloccol where n is checked carefully, or from shallow() at R level
   // where n is set to truelength (i.e. a shallow copy only with no size change)
   int protecti=0;
-  SEXP newdt = PROTECT(allocVector(VECSXP, n)); protecti++;   // to do, use growVector here?
+  SEXP newdt = PROTECT(R_allocResizableVector(VECSXP, n)); protecti++;   // to do, use growVector here?
   SHALLOW_DUPLICATE_ATTRIB(newdt, dt);
 
   // TO DO: keepattr() would be faster, but can't because shallow isn't merely a shallow copy. It
@@ -169,7 +171,7 @@ static SEXP shallow(SEXP dt, SEXP cols, R_len_t n)
   setAttrib(newdt, sym_sorted, duplicate(sorted));
 
   SEXP names = PROTECT(getAttrib(dt, R_NamesSymbol)); protecti++;
-  SEXP newnames = PROTECT(allocVector(STRSXP, n)); protecti++;
+  SEXP newnames = PROTECT(R_allocResizableVector(STRSXP, n)); protecti++;
   const int l = isNull(cols) ? LENGTH(dt) : length(cols);
   if (isNull(cols)) {
     for (int i=0; i<l; ++i) SET_VECTOR_ELT(newdt, i, VECTOR_ELT(dt,i));
@@ -186,13 +188,9 @@ static SEXP shallow(SEXP dt, SEXP cols, R_len_t n)
       for (int i=0; i<l; ++i) SET_STRING_ELT( newnames, i, STRING_ELT(names,INTEGER(cols)[i]-1) );
     }
   }
+  R_resizeVector(newdt,l);
+  R_resizeVector(newnames,l);
   setAttrib(newdt, R_NamesSymbol, newnames);
-  // setAttrib appears to change length and truelength, so need to do that first _then_ SET next,
-  // otherwise (if the SET were were first) the 100 tl is assigned to length.
-  SETLENGTH(newnames,l);
-  SET_TRUELENGTH(newnames,n);
-  SETLENGTH(newdt,l);
-  SET_TRUELENGTH(newdt,n);
   setselfref(newdt);
   UNPROTECT(protecti);
   return(newdt);
@@ -527,8 +525,9 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
       internal_error(__func__, "selfrefnames is ok but tl names [%lld] != tl [%d]", (long long)TRUELENGTH(names), oldtncol);  // # nocov
     if (!selfrefok(dt, verbose)) // #6410 setDT(dt) and subsequent attr<- can lead to invalid selfref
       error(_("It appears that at some earlier point, attributes of this data.table have been reassigned. Please use setattr(DT, name, value) rather than attr(DT, name) <- value. If that doesn't apply to you, please report your case to the data.table issue tracker."));
-    SETLENGTH(dt, oldncol+LENGTH(newcolnames));
-    SETLENGTH(names, oldncol+LENGTH(newcolnames));
+    R_resizeVector(dt, oldncol+LENGTH(newcolnames));
+    R_resizeVector(names, oldncol+LENGTH(newcolnames));
+    setAttrib(dt, R_NamesSymbol, names);
     for (int i=0; i<LENGTH(newcolnames); ++i)
       SET_STRING_ELT(names,oldncol+i,STRING_ELT(newcolnames,i));
     // truelengths of both already set by alloccol
@@ -726,8 +725,9 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
       SET_VECTOR_ELT(dt, i, R_NilValue);
       SET_STRING_ELT(names, i, NA_STRING);  // release reference to the CHARSXP
     }
-    SETLENGTH(dt,    ndt-ndelete);
-    SETLENGTH(names, ndt-ndelete);
+    R_resizeVector(dt,    ndt-ndelete);
+    R_resizeVector(names, ndt-ndelete);
+    setAttrib(dt, R_NamesSymbol, names);
     if (LENGTH(names)==0) {
       // That was last column deleted, leaving NULL data.table, so we need to reset .row_names, so that it really is the NULL data.table.
       PROTECT(nullint=allocVector(INTSXP, 0)); protecti++;

src/dogroups.c

@@ -322,8 +322,9 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
           // Even if we could know reliably to switch from allocNAVectorLike to allocVector for slight speedup, user code could still
           // contain a switched halt, and in that case we'd want the groups not yet done to have NA rather than 0 or uninitialized.
           // Increment length only if the allocation passes, #1676. But before SET_VECTOR_ELT otherwise attempt-to-set-index-n/n R error
-          SETLENGTH(dtnames, LENGTH(dtnames)+1);
-          SETLENGTH(dt, LENGTH(dt)+1);
+          R_resizeVector(dtnames, LENGTH(dtnames)+1);
+          R_resizeVector(dt, LENGTH(dt)+1);
+          setAttrib(dt, R_NamesSymbol, dtnames);
           SET_VECTOR_ELT(dt, colj, target);
           UNPROTECT(1);
           SET_STRING_ELT(dtnames, colj, STRING_ELT(newnames, colj-origncol));
@@ -534,7 +535,7 @@ SEXP growVector(SEXP x, const R_len_t newlen)
   SEXP newx;
   R_len_t len = length(x);
   if (isNull(x)) error(_("growVector passed NULL"));
-  PROTECT(newx = allocVector(TYPEOF(x), newlen));   // TO DO: R_realloc(?) here?
+  PROTECT(newx = R_allocResizableVector(TYPEOF(x), newlen));   // TO DO: R_realloc(?) here?
   if (newlen < len) len=newlen;   // i.e. shrink
   if (!len) { // cannot memcpy invalid pointer, #6819
     keepattr(newx, x);

src/freadR.c

@@ -265,8 +265,8 @@ bool userOverride(int8_t *type, lenOff *colNames, const char *anchor, const int
   // use typeSize superfluously to avoid not-used warning; otherwise could move typeSize from fread.h into fread.c
   if (typeSize[CT_BOOL8_N] != 1) internal_error(__func__, "typeSize[CT_BOOL8_N] != 1"); // # nocov
   if (typeSize[CT_STRING] != 8) internal_error(__func__, "typeSize[CT_STRING] != 8"); // # nocov
-  colNamesSxp = R_NilValue;
-  SET_VECTOR_ELT(RCHK, 1, colNamesSxp = allocVector(STRSXP, ncol));
+  colNamesSxp = R_allocResizableVector(STRSXP, ncol);
+  SET_VECTOR_ELT(RCHK, 1, colNamesSxp);
   for (int i = 0; i < ncol; i++) {
     if (colNames == NULL || colNames[i].len <= 0) {
       char buff[12];
@@ -452,7 +452,8 @@ size_t allocateDT(int8_t *typeArg, int8_t *sizeArg, int ncolArg, int ndrop, size
   if (newDT) {
     ncol = ncolArg;
     dtnrows = allocNrow;
-    SET_VECTOR_ELT(RCHK, 0, DT = allocVector(VECSXP, ncol - ndrop));
+    DT = R_allocResizableVector(VECSXP, ncol - ndrop);
+    SET_VECTOR_ELT(RCHK, 0, DT);
     if (ndrop == 0) {
       setAttrib(DT, R_NamesSymbol, colNamesSxp);  // colNames mkChar'd in userOverride step
       if (colClassesAs) setAttrib(DT, sym_colClassesAs, colClassesAs);
@@ -512,8 +513,8 @@ size_t allocateDT(int8_t *typeArg, int8_t *sizeArg, int ncolArg, int ndrop, size
     const bool typeChanged = (type[i] > 0) && (newDT || TYPEOF(col) != typeSxp[type[i]] || oldIsInt64 != newIsInt64);
     const bool nrowChanged = (allocNrow != dtnrows);
     if (typeChanged || nrowChanged) {
-      SEXP thiscol = typeChanged ? allocVector(typeSxp[type[i]], allocNrow) : growVector(col, allocNrow); // no need to PROTECT, passed immediately to SET_VECTOR_ELT, see R-exts 5.9.1
-      
+      SEXP thiscol = typeChanged ? R_allocResizableVector(typeSxp[type[i]], allocNrow) : growVector(col, allocNrow); // no need to PROTECT, passed immediately to SET_VECTOR_ELT, see R-exts 5.9.1
+
       SET_VECTOR_ELT(DT, resi, thiscol);
       if (type[i] == CT_INT64) {
         SEXP tt = PROTECT(ScalarString(char_integer64));
@@ -534,7 +535,6 @@ size_t allocateDT(int8_t *typeArg, int8_t *sizeArg, int ncolArg, int ndrop, size
 
         setAttrib(thiscol, sym_tzone, ScalarString(char_UTC)); // see news for v1.13.0
       }
-      SET_TRUELENGTH(thiscol, allocNrow);
       DTbytes += RTYPE_SIZEOF(thiscol) * allocNrow;
     }
     resi++;
@@ -551,9 +551,7 @@ void setFinalNrow(size_t nrow)
       return;
     const int ncol = LENGTH(DT);
     for (int i = 0; i < ncol; i++) {
-      SETLENGTH(VECTOR_ELT(DT, i), nrow);
-      SET_TRUELENGTH(VECTOR_ELT(DT, i), dtnrows);
-      SET_GROWABLE_BIT(VECTOR_ELT(DT, i));  // #3292
+      R_resizeVector(VECTOR_ELT(DT, i), nrow);
     }
   }
   R_FlushConsole(); // # 2481. Just a convenient place; nothing per se to do with setFinalNrow()
@@ -567,8 +565,9 @@ void dropFilledCols(int* dropArg, int ndelete)
     SET_VECTOR_ELT(DT, dropFill[i], R_NilValue);
     SET_STRING_ELT(colNamesSxp, dropFill[i], NA_STRING);
   }
-  SETLENGTH(DT, ndt - ndelete);
-  SETLENGTH(colNamesSxp, ndt - ndelete);
+  R_resizeVector(DT, ndt - ndelete);
+  R_resizeVector(colNamesSxp, ndt - ndelete);
+  setAttrib(DT, R_NamesSymbol, colNamesSxp);  // reinstall after resize
 }
 
 void pushBuffer(ThreadLocalFreadParsingContext *ctx)

src/frollapply.c

@@ -50,7 +50,7 @@ SEXP memcpyVectoradaptive(SEXP dest, SEXP src, SEXP offset, SEXP size) {
   const size_t oi = INTEGER_RO(offset)[0];
   const int nrow = INTEGER_RO(size)[oi - 1];
   const size_t o = oi - nrow; // oi should always be bigger than nrow because we filter out incomplete window using ansMask
-  SETLENGTH(dest, nrow); // must be before memcpy_sexp because attempt to set index 1/1 in SET_STRING_ELT test 6010.150
+  R_resizeVector(dest, nrow); // must be before memcpy_sexp because attempt to set index 1/1 in SET_STRING_ELT test 6010.150
   if (nrow) { // support k[i]==0
     memcpy_sexp(dest, o, src, nrow);
   }
@@ -65,7 +65,7 @@ SEXP memcpyDTadaptive(SEXP dest, SEXP src, SEXP offset, SEXP size) {
   const int ncol = LENGTH(dest);
   for (int i = 0; i < ncol; i++) {
     SEXP d = VECTOR_ELT(dest, i);
-    SETLENGTH(d, nrow);
+    R_resizeVector(d, nrow);
     if (nrow) { // support k[i]==0
       memcpy_sexp(d, o, VECTOR_ELT(src, i), nrow);
     }
@@ -76,18 +76,5 @@ SEXP memcpyDTadaptive(SEXP dest, SEXP src, SEXP offset, SEXP size) {
 
 // needed in adaptive=TRUE
 SEXP setgrowable(SEXP x) {
-  if (!isNewList(x)) {
-    SET_GROWABLE_BIT(x);
-    SET_TRUELENGTH(x, LENGTH(x)); // important because gc() uses TRUELENGTH to keep counts
-  } else {
-    // # nocov start ## does not seem to be reported to codecov most likely due to running in a fork, I manually debugged that it is being called when running froll.Rraw
-    for (int i = 0; i < LENGTH(x); i++) {
-      //Rprintf("%d",3); // manual code coverage to confirm it is reached when marking nocov
-      SEXP col = VECTOR_ELT(x, i);
-      SET_GROWABLE_BIT(col);
-      SET_TRUELENGTH(col, LENGTH(col));
-    }
-    // # nocov end
-  }
   return x;
 }

src/subset.c

@@ -297,16 +297,15 @@ SEXP subsetDT(SEXP x, SEXP rows, SEXP cols) { // API change needs update NEWS.md
   }
 
   int overAlloc = checkOverAlloc(GetOption1(install("datatable.alloccol")));
-  SEXP ans = PROTECT(allocVector(VECSXP, LENGTH(cols)+overAlloc)); nprotect++;  // doing alloc.col directly here; eventually alloc.col can be deprecated.
+  SEXP ans = PROTECT(R_allocResizableVector(VECSXP, LENGTH(cols)+overAlloc)); nprotect++;  // doing alloc.col directly here; eventually alloc.col can be deprecated.
 
   // user-defined and superclass attributes get copied as from v1.12.0
   copyMostAttrib(x, ans);
   // most means all except R_NamesSymbol, R_DimSymbol and R_DimNamesSymbol
   // includes row.names (oddly, given other dims aren't) and "sorted" dealt with below
   // class is also copied here which retains superclass name in class vector as has been the case for many years; e.g. tests 1228.* for #64
 
-  SET_TRUELENGTH(ans, LENGTH(ans));
-  SETLENGTH(ans, LENGTH(cols));
+  R_resizeVector(ans, LENGTH(cols));
   int ansn;
   if (isNull(rows)) {
     ansn = nrow;
@@ -329,9 +328,8 @@ SEXP subsetDT(SEXP x, SEXP rows, SEXP cols) { // API change needs update NEWS.md
       subsetVectorRaw(target, source, rows, anyNA);  // parallel within column
     }
   }
-  SEXP tmp = PROTECT(allocVector(STRSXP, LENGTH(cols)+overAlloc)); nprotect++;
-  SET_TRUELENGTH(tmp, LENGTH(tmp));
-  SETLENGTH(tmp, LENGTH(cols));
+  SEXP tmp = PROTECT(R_allocResizableVector(STRSXP, LENGTH(cols)+overAlloc)); nprotect++;
+  R_resizeVector(tmp, LENGTH(cols));
   setAttrib(ans, R_NamesSymbol, tmp);
   subsetVectorRaw(tmp, getAttrib(x, R_NamesSymbol), cols, /*anyNA=*/false);