copy(): only overwrite the list when necessary, consider the attributes

[aitap]

Rewrite copy() in C so that instead of calling y[] <- lapply(y, reallocate) we can consider each individual list element and only call SET_VECTOR_ELT when necessary. This avoids the problem with xgboost's special altlist that must never be altered.

Also walk the attributes of the object being copied, look for data.tables and reallocate them. Maybe that's not necessary (copy() has been working fine without it) and can be taken out.

Test 2224.3 points out an important detail: when y is a LISTSXP pairlist, y[] <- lapply(y, reallocate) makes it into a VECSXP list instead. It turns out we've been relying on copy() automatically coercing pairlist columns into vector lists when constructing data.tables. Should this become more explicit and be moved somewhere into the data.table() constructor, leaving copy() to just duplicate + reallocate data.tables?

Unfortunately this is hard to test without loading xgboost or creating a different altclass.

Fixes: #7456

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (94649d5) to head (cf6449a).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7820   +/-   ##
=======================================
  Coverage   99.01%   99.02%           
=======================================
  Files          88       88           
  Lines       17234    17250   +16     
=======================================
+ Hits        17065    17081   +16     
  Misses        169      169           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

• HEAD=fix7456 much slower for transform improved in #5493
  

Generated via commit cf6449a

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	3 minutes and 27 seconds
Installing different package versions	24 seconds
Running and plotting the test cases	5 minutes and 29 seconds

[ben-schwen]

What immediately comes to my mind. How fast is this in terms of speed?

[ben-schwen]

Suggested change

	static SEXP realloc_nested_dt_list(SEXP x, int overAlloc, bool);
	static SEXP realloc_nested_dt_list(SEXP x, int overAlloc, bool replacePairlists);

or dont name arguments at all.

[ben-schwen]

should we add a test for nested tables?

inner = data.table(a=1)
DT = data.table(x=1:2, y=list(inner, inner))
DT2 = copy(DT)
test(2379.1, truelength(DT2$y[[1L]]) > 0L)  # is FALSE in current master
test(2379.2, DT2$y[[1L]][, b := 2][, b], 2) # warns in current master

[ben-schwen]

what if we have multiple attributes like:

x = list()
attr(x, 'foo') = 1:2
attr(x, 'dt') = data.table()
truelength(attr(copy(x), 'dt'))

[aitap]

atime::atime_versions(
 '.', N = 10^seq(0, 3, length.out = 10),
 setup = x <- (\(n) if (n > 0) list(Recall(n-1)) else replicate(100, vector('list', 100), FALSE) |>
as.data.table())(N),
 expr = data.table::copy(x),
 sha.vec = c(master = 'master', Ccopy = 'fix7456', 'no att' = 'fix7456noatt'),
 seconds.limit = 3, times = 20,
 pkg.edit.fun = '.ci/atime/tests.R' |> parse() |> _[[6]][[3]]$pkg.edit.fun |> eval()
) -> ret

Strange. With a contrived example I can show the new `copy` being faster, but there's a constant-factor slowdown unrelated to attribute scanning. Will have to investigate more.

[ben-schwen]

Not sure how atime works internally, but could this be because of some C interface overhead?

[ben-schwen]

Change LGTM. Might want to look deeper into timings before merging though