uniqueN escape forder to base R for small atomic vectors, #1120

[jangorecki]

this address some of the slowest uses cases of uniqueN by make groups
related #3739, #1120, #3725, #3438

[codecov]

Codecov Report

Merging #3743 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #3743      +/-   ##
=========================================
- Coverage   99.41%   99.4%   -0.01%     
=========================================
  Files          71      71              
  Lines       13208   13214       +6     
=========================================
+ Hits        13131   13136       +5     
- Misses         77      78       +1

Impacted Files	Coverage Δ
R/duplicated.R	98.48% <100%> (-1.52%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d959107...410843a. Read the comment docs.

[HughParsonage]

Probably slightly quicker to not na.omit but just do:

return(length(x) - sum(duplicated(x)) - {na.rm && anyNA(x)})

[jangorecki]

I am not getting how that would work

[HughParsonage]

If na.rm = FALSE then my version is just length(x) - sum(duplicated(x)) - 0.

If na.rm = TRUE then it will be length(x) - sum(duplicated(x)) - 1 * anyNA(x).

[MichaelChirico]

(key insight being that uniqueN can differ by at most 1 for na.rm vs !na.rm)

[jangorecki]

nice one!
it seems that it might eventually differ by 2

> x=c(1,2,NA,2,3,NaN,NaN,NA)
> length(unique(x))
[1] 5
> uniqueN(x)
[1] 5
> uniqueN(x, na.rm=T)
[1] 3
> length(unique(na.omit(x)))
[1] 3
> uniqueN(na.omit(x))
[1] 3

[mattdowle]

This is good and clearly a problem that should be fixed. But could this switch for small sizes be in forder.c? If so, the problem in forder.c should really be fixed there rather than putting a fix into duplicated.R only. That way we fix the root cause at the root, with uniqueN being one use case.
A few quick candidates by eye:

1. https://github.com/Rdatatable/data.table/blob/master/src/forder.c#L476
   If there's overhead in using a parallel team, that's the first place and would always be done regardless of vector length.

2. insert sort (non-parallel) kicks in under 256 items: https://github.com/Rdatatable/data.table/blob/master/src/forder.c#L804

3. there's another threshold at UINT16_MAX (65,535) : https://github.com/Rdatatable/data.table/blob/master/src/forder.c#L917

If forder.c is more closely profiled (more TBEG, TEND needed) then we can see exactly where the root slowdown in forder.c is. It's also a good excuse to review forder.c.