Use %chin% subsetting instead of setdiff(intersect()) for j/by col detection

[m-muecke]

intersect, setdiff etc. is very slow for chars, in other projects (mlr3), we use %in% instead:

  # faster intersect, setdiff etc. that rely on x, y being characters with no duplicates
  char_intersect = function(x, y) x[x %in% y]
  char_setdiff = function(x, y) x[!x %in% y]
  char_union = function(x, y) c(x, y[!y %in% x])

a rough local benchmark for the isolated function change:

n per vector	intersect	setdiff	union
5	10.3µs → 0.90µs (11.4x)	10.1µs → 0.98µs (10.3x)	10.9µs → 1.11µs (9.9x)
50	12.4µs → 1.89µs (6.6x)	12.1µs → 1.89µs (6.4x)	13.4µs → 2.38µs (5.6x)
1,000	60.2µs → 23.4µs (2.6x)	63.6µs → 27.5µs (2.3x)	69.9µs → 32.8µs (2.1x)
100,000	10.3ms → 5.29ms (1.9x)	9.38ms → 4.80ms (2.0x)	9.88ms → 5.20ms (1.9x)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (c838f30) to head (2ddfa74).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7913   +/-   ##
=======================================
  Coverage   99.02%   99.02%           
=======================================
  Files          88       88           
  Lines       17387    17388    +1     
=======================================
+ Hits        17217    17218    +1     
  Misses        170      170           

☔ 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.

[jangorecki]

Thanks, benchmarks you presented is not really much of use. Could you provide benchmark of [.data.table that shows improvements? ideally most optimistic and most pessimistic cases.

[m-muecke]

makes sense, the larger the n the less it has an impact (all done via bench::mark()):

case	master	PR	saved
optimistic (100 rows)
DT[, .N, by = .(g, h)]	186.2µs	154.2µs	32.0µs	17.2% faster
DT[, sum(v), by = g]	218.1µs	181.4µs	36.7µs	16.8% faster
DT[v > 0, w]	110.2µs	94.3µs	15.9µs	14.5% faster
DT[, .(v, w)]	188.1µs	172.1µs	16.1µs	8.5% faster
DT[, lapply(.SD, sum), by = g]	277.7µs	261.4µs	16.4µs	5.9% faster
cube(DT, j = sum(v), by = c("g", "h"))	1338.0µs	1205.9µs	132.1µs	9.9% faster
W[, sum(V1), by = g], 10,000 cols	936.0µs	577.5µs	358.5µs	38.3% faster
pessimistic (1e7 rows)
L[, sum(v), by = g]	116.4ms	113.9ms		within noise

DT[, sum(V1), by = g] with varying size:

rows (10 cols):

rows	master	PR	saved
100	218.8µs	179.1µs	39.7µs	18.2% faster
10,000	350.2µs	321.0µs	29.2µs	8.3% faster
1e6	14.62ms	14.69ms		within noise
1e7	149.6ms	150.5ms		within noise

cols (100 rows):

columns	master	PR	saved
10	218.6µs	178.4µs	40.2µs	18.4% faster
100	223.1µs	183.0µs	40.1µs	18.0% faster
1,000	284.5µs	222.7µs	61.7µs	21.7% faster
10,000	945.1µs	574.8µs	370.4µs	39.2% faster

Apple M2 Pro (arm64), macOS 27.0.1, R 4.6.1, Homebrew clang 23.1.3

[jangorecki]

intersect and setdiff will handle differently duplicates in the input than %chin% would.
I would assume %chin% would be preferred behavior in most cases, although not necessarily here. It would be good to have unit tests covering duplicates too.

[m-muecke]

Duplicates are only relevant for the left-hand side, and here all places come from all.vars() which already does deduplication. Or do you mean something different?

[jangorecki]

uh, good point abt all.vars(), we should be safe then