tidyverse / vroom

Fast reading of delimited files
https://vroom.r-lib.org
Other
622 stars 60 forks source link

CRAN checks and vroom #527

Closed bart1 closed 9 months ago

bart1 commented 9 months ago

When checking my package that depends on vroom (move2) with win-builder I frequently get notes about examples taking to long. As far as I can see this relates to a function that uses vroom to read a gz compressed example csv file (700 kb). Due to vroom attempting to use many cores the user time gets quite long even thought the elapsed time is more then short enough. This could naturally be solved by setting num_threads to 1, however this generally makes a poor example (in most cases users should not bother with 'num_threads'). As far as I can see there is no option to set VROOM_THREADS in the (CRAN) testing environment. Is there any solution I'm missing to speed up these examples that are based on vroom? Or should I forcefully set the number of threads to 1 everywhere? Sorry for asking this question that is not really a bug here but it seemed the most suitable place where others might also profit from it.

Examples with CPU (user + system) or elapsed time > 10s
                            user system elapsed
mt_example                 26.90   0.42    1.32
mt_filter_unique           26.09   0.56    2.45
mt_read                    25.76   0.55    1.66
mt_aeqd_crs                25.35   0.46    2.17
mt_filter_movebank_visible 25.41   0.38    1.41
jennybc commented 9 months ago

I would probably use the @examplesIf technique described here to check for whatever you consider to be favorable conditions for executing the affected example.

https://r-pkgs.org/man.html#sec-man-examples-dependencies-conditional-execution

bart1 commented 9 months ago

@jennybc thank you, I did not think of that. This currently works:

#' @examplesIf parallel::detectCores() < 9

Note I avoid vroom_threads() as it is not exported, alternatively a similar construct could be set up to temporarily set VROOM_THREADS in dontshow. However this seems to work for the time being