Skip to content

Commit 2fa7728

Browse files
committed
Drop the finalizer
Now that (1) we depend on R >= 3.4 and (2) data.table objects have the GROWABLE_BIT set, there is no need to adjust allocated memory counts by hand.
1 parent 04437b6 commit 2fa7728

File tree

1 file changed

+2
-59
lines changed

1 file changed

+2
-59
lines changed

src/assign.c

Lines changed: 2 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,5 @@
11
#include "data.table.h"
22

3-
static void finalizer(SEXP p)
4-
{
5-
R_len_t n, l, tl;
6-
if(!R_ExternalPtrAddr(p)) internal_error(__func__, "didn't receive an ExternalPtr"); // # nocov
7-
p = R_ExternalPtrTag(p);
8-
if (!isString(p)) internal_error(__func__, "ExternalPtr doesn't see names in tag"); // # nocov
9-
l = LENGTH(p);
10-
tl = TRUELENGTH(p);
11-
if (l<0 || tl<l) internal_error(__func__, "l=%d, tl=%d", l, tl); // # nocov
12-
n = tl-l;
13-
if (n==0) {
14-
// gc's ReleaseLargeFreeVectors() will have reduced R_LargeVallocSize by the correct amount
15-
// already, so nothing to do (but almost never the case).
16-
return;
17-
}
18-
#ifdef DODO
19-
SEXP x;
20-
x = PROTECT(allocVector(INTSXP, 50)); // 50 so it's big enough to be on LargeVector heap. See NodeClassSize in memory.c:allocVector
21-
// INTSXP rather than VECSXP so that GC doesn't inspect contents after LENGTH (thanks to Karl Millar, Jul 2015)
22-
SETLENGTH(x,50+n*2*sizeof(void *)/4); // 1*n for the names, 1*n for the VECSXP itself (both are over allocated).
23-
#endif
24-
UNPROTECT(1);
25-
return;
26-
}
27-
283
void setselfref(SEXP x) {
294
if(!INHERITS(x, char_datatable)) return; // #5286
305
SEXP p;
@@ -40,7 +15,6 @@ void setselfref(SEXP x) {
4015
R_NilValue
4116
))
4217
));
43-
R_RegisterCFinalizerEx(p, finalizer, FALSE);
4418
UNPROTECT(2);
4519

4620
/*
@@ -66,39 +40,8 @@ void setselfref(SEXP x) {
6640
*/
6741
}
6842

69-
/* There are two reasons the finalizer doesn't restore the LENGTH to TRUELENGTH. i) The finalizer
70-
happens too late after GC has already released the memory, and ii) copies by base R (e.g.
71-
[<- in write.table just before test 894) allocate at length LENGTH but copy the TRUELENGTH over.
72-
If the finalizer sets LENGTH to TRUELENGTH, that's a fail as it wasn't really allocated at
73-
TRUELENGTH when R did the copy.
74-
Karl Millar suggested an ENVSXP so that restoring LENGTH in finalizer should work. This is the
75-
closest I got to getting it to pass all tests :
76-
77-
SEXP env = PROTECT(allocSExp(ENVSXP));
78-
defineVar(SelfRefSymbol, x, env);
79-
defineVar(R_NamesSymbol, getAttrib(x, R_NamesSymbol), env);
80-
setAttrib(x, SelfRefSymbol, p = R_MakeExternalPtr(
81-
R_NilValue, // for identical() to return TRUE. identical() doesn't look at tag and prot
82-
R_NilValue, //getAttrib(x, R_NamesSymbol), // to detect if names has been replaced and its tl lost, e.g. setattr(DT,"names",...)
83-
PROTECT( // needed when --enable-strict-barrier it seems, iiuc. TO DO: test under that flag and remove if not needed.
84-
env // wrap x in env to avoid an infinite loop in object.size() if prot=x were here
85-
)
86-
));
87-
R_RegisterCFinalizerEx(p, finalizer, FALSE);
88-
UNPROTECT(2);
89-
90-
Then in finalizer:
91-
SETLENGTH(names, tl)
92-
SETLENGTH(dt, tl)
93-
94-
and that finalizer indeed now happens before the GC releases memory (thanks to the env wrapper).
95-
96-
However, we still have problem (ii) above and it didn't pass tests involving base R copies.
97-
98-
We really need R itself to start setting TRUELENGTH to be the allocated length and then
99-
for GC to release TRUELENGTH not LENGTH. Would really tidy this up.
100-
101-
Moved out of ?setkey Details section in 1.12.2 (Mar 2019). Revisit this w.r.t. to recent versions of R.
43+
/*
44+
Moved out of ?setkey Details section in 1.12.2 (Mar 2019). Revisit this w.r.t. to recent versions of R.
10245
The problem (for \code{data.table}) with the copy by \code{key<-} (other than
10346
being slower) is that \R doesn't maintain the over-allocated truelength, but it
10447
looks as though it has. Adding a column by reference using \code{:=} after a

0 commit comments

Comments
 (0)