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.