krlmlr · GitHub

It's already there: with_hybrid(max(a), a = 1:5) vs. without_hybrid((max)(a), a = 1:5). (Just fixed a few problems in master.)

Current master

library(dplyr, warn.conflicts = FALSE)
set.seed(123)
N <- 1e7
x <- runif(N)
xx <- x
xx[N/2] <- NA
microbenchmark::microbenchmark(
  dplyr:::with_hybrid(max(x), x),
  dplyr:::without_hybrid((max)(x), x),
  dplyr:::with_hybrid(max(xx), xx),
  dplyr:::without_hybrid((max)(xx), xx),
  dplyr:::with_hybrid(max(xx, na.rm = TRUE), xx),
  dplyr:::without_hybrid((max)(xx, na.rm = TRUE), xx),
  times = 20
)
#> Unit: milliseconds
#>                                                 expr      min       lq
#>                       dplyr:::with_hybrid(max(x), x) 55.48891 57.39561
#>                  dplyr:::without_hybrid((max)(x), x) 13.54115 14.00521
#>                     dplyr:::with_hybrid(max(xx), xx) 27.96920 29.19869
#>                dplyr:::without_hybrid((max)(xx), xx) 13.69948 14.61733
#>       dplyr:::with_hybrid(max(xx, na.rm = TRUE), xx) 55.02973 55.85497
#>  dplyr:::without_hybrid((max)(xx, na.rm = TRUE), xx) 13.81365 14.53460
#>      mean   median       uq      max neval cld
#>  60.72320 60.20266 63.49677 67.44705    20   c
#>  16.04345 15.21578 17.77672 22.53380    20 a  
#>  31.27390 30.79920 31.78185 43.49719    20  b 
#>  15.47042 15.32329 15.60884 20.75653    20 a  
#>  61.45400 58.53612 65.52276 77.96244    20   c
#>  18.34514 15.17433 17.60567 43.77499    20 a

This PR

#> Unit: milliseconds
#>                                                 expr      min       lq
#>                       dplyr:::with_hybrid(max(x), x) 60.63637 61.25377
#>                  dplyr:::without_hybrid((max)(x), x) 13.55323 14.14427
#>                     dplyr:::with_hybrid(max(xx), xx) 31.07457 31.26109
#>                dplyr:::without_hybrid((max)(xx), xx) 13.41300 13.94740
#>       dplyr:::with_hybrid(max(xx, na.rm = TRUE), xx) 54.90483 55.64433
#>  dplyr:::without_hybrid((max)(xx, na.rm = TRUE), xx) 13.36041 13.85364
#>      mean   median       uq      max neval  cld
#>  62.70054 62.29523 64.12417 66.02901    20    d
#>  14.92565 14.73436 15.01480 21.73895    20 a   
#>  32.84819 32.19134 32.93270 45.22439    20  b  
#>  14.28596 14.29246 14.68599 14.98137    20 a   
#>  57.13864 56.20387 58.92122 59.57113    20   c 
#>  14.43992 14.24315 14.71131 17.95994    20 a

The na.rm = FALSE case has become a tad slower with this change (with gcc -O2 on my laptop), overall our implementation is slower than base R's by a factor of ~3-4. However, base R doesn't seem to have a shortcut for na.rm = FALSE

I combined the four classes into one to reduce maintenance effort (now and in the future). I'd argue that a minimal run time improvement isn't worth the extra maintenance effort, given that we're already sub-par. I could look at a profiler run if it's important, though.

Read the original on github.com ↗