`:=` doesn't always assign in-place

[eantonya]

dt = data.table(a = 1:3)
address(dt$a)
#[1] "000000000EB62368"

# good
dt[1, a := a + a]
address(dt$a)
#[1] "000000000EB62368"

# good
dt[1:3, a := a + a]
address(dt$a)
#[1] "000000000EB62368"

# bad
dt[, a := a + a]
address(dt$a)
#[1] "000000000E6F2930"

[nkurz]

# also good
dt[1:.N, a := a + a]

[eantonya]

Since I assume we all would agree that dt[TRUE,...] should be equivalent to dt[ , ...] a super-simple patch for this is adding if (missing(i)) i = TRUE at the top of [.data.table or better yet, setting TRUE to be the default value of i and getting rid of the missing checks.

Of course assign would also need to assume that i always exists/have slightly modified logic for changing column types.

[nkurz]

The cure of switching to dt[TRUE, ...] turns out to be significantly worse than the problem with dt[ , ...]. With nrow(dt) == N, if i is missing we do not update in place, but we only do a single 8N byte allocation. With i=TRUE, R 3.2.1 with current Github master does a discouraging 28N bytes of allocation!

I'll file a separate bug for this, but if the goal is avoiding unnecessary copies and allocations and in-place changes are not a requirement, using dt[TRUE, ...] is not an acceptable workaround. Or rather, while i=TRUE should be made equivalent to a missing i (or vice versa), they both need to be fixed.

Concentrating here only on the case of a missing i, the 8N allocation we want to avoid is at line 1196:

jval = eval(jsub, SDenv, parent.frame())

Since this is happening in an eval(), presumably the problem is that the column being changed has multiple references? Or had them in the past and base R is being conservative and copying first?

[eantonya]

@nkurz I have no idea what the 3.5x allocation is about - please file a separate issue for that. For this particular issue I have a semi-working version now, but all of the modifications are in the C code.

I gave up on the literal i=TRUE idea, because it would slow down the missing i case quite a bit. From my pov as long as missing i is exactly same as i=TRUE (or any other complicated expression that selects all of the data.table), everything is fine, and it's irrelevant what the underlying code actually does.

[nkurz]

I filed the other bug at #1249. While it is true that a literal i=TRUE would slow down the current code, it's a very clear approach. The cleanest solution might be to coerce to i=TRUE, and then make that a fast path never creating the full logical vector. I know C much better than I know R, and I can test your fix, but my lack of familiarity with the internals of R prevents me from proposing any fixes myself.

[eantonya]

Yes, I agree that would be cleaner, and generally speaking [.data.table is long overdue for a complete rehaul and cleanup, but I'm not going to volunteer for that job just yet :) My patch is going to be somewhat orthogonal to having i=TRUE being the default, so that can be added at a later point.

[mattdowle]

Well spotted. As suggested, fixed by converting i=TRUE to i=missing.

Instead of 5 column allocatons, there's just one now for the a+a expression (the RHS, which gets created anyway) which is then plonked into the column slot by reference i.e. address(DT) doesn't change but address(DT$a) will change. That's correct behaviour, and most efficient, to save copying the whole RHS into the existing column (which is only possible if they're the same type anyway). Since the RHS is as long as the number of rows, it is just plonked in. There's more on plonk in ?":=".

Closing for now but please reopen if I missed something.