Skip to content

Remove check for primitive/builtin in as_mapper.default() to improve performance#1218

Merged
shikokuchuo merged 5 commits intotidyverse:mainfrom
mtcarsalot:mapMapperDefault
Sep 22, 2025
Merged

Remove check for primitive/builtin in as_mapper.default() to improve performance#1218
shikokuchuo merged 5 commits intotidyverse:mainfrom
mtcarsalot:mapMapperDefault

Conversation

@mtcarsalot
Copy link
Contributor

@mtcarsalot mtcarsalot commented Sep 19, 2025

  • change test to reflect
  • The func description did not mention primitives, so i did not explain that these are handled differently now...

Fixes #1088

performance before and after fix.

x <- as.list(1:20)
x[21] <- list(NULL)

bench::mark(
   wrapped = purrr::detect_index(x, function(x) is.null(x)),
   prim = purrr::detect_index(x, is.null)
)
# A tibble: 2 × 13
  expression      min   median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result    memory              time       gc      
  <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>    <list>              <list>     <list>  
1 wrapped      62.3µs   65.6µs    13760.    4.12KB     6.29  6563     3      477ms <int [1]> <Rprofmem [12 × 3]> <bench_tm> <tibble>
2 prim        137.4µs  143.7µs     6502.        0B     6.28  3108     3      478ms <int [1]> <Rprofmem [0 × 3]>  <bench_tm> <tibble>

 devtools::load_all()
ℹ Loading purrr

bench::mark(
   wrapped = purrr::detect_index(x, function(x) is.null(x)),
   prim = purrr::detect_index(x, is.null) 
)
# A tibble: 2 × 13
  expression      min   median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result    memory              time       gc      
  <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>    <list>              <list>     <list>  
1 wrapped        75µs   78.1µs    12529.    4.12KB     6.31  5955     3      475ms <int [1]> <Rprofmem [12 × 3]> <bench_tm> <tibble>
2 prim         69.3µs   72.7µs    13506.    30.8KB     8.52  6338     4      469ms <int [1]> <Rprofmem [68 × 3]> <bench_tm> <tibble>

@hadley
Copy link
Member

hadley commented Sep 19, 2025

Could you please add a news bullet too?

@shikokuchuo shikokuchuo changed the title remove check for pimitive/builtin in as_mapper.default() to improve performance Remove check for primitive/builtin in as_mapper.default() to improve performance Sep 22, 2025
@shikokuchuo
Copy link
Member

Thanks for your contribution @mtcarsalot! I just made a small edit to the news item and added a comment to the test to make it clear that the current behaviour is intentional.

@shikokuchuo shikokuchuo merged commit b566017 into tidyverse:main Sep 22, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

as_mapper.default() is relatively slow

4 participants