Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests parallelizes with all available CPU cores #56

Open
HenrikBengtsson opened this issue Dec 21, 2023 · 3 comments
Open

Tests parallelizes with all available CPU cores #56

HenrikBengtsson opened this issue Dec 21, 2023 · 3 comments

Comments

@HenrikBengtsson
Copy link

When running revdep checks on future, I noticed that the unit tests of arkdb end up spawning N parallel processes, where N = all CPU cores on the machine. For example, here is what I observe when I run on a host with 48 cores;

Screenshot from 2023-12-21 13-09-09

Here's what I managed to grab using pstree:

 |-sh /wynton/home/cbi/shared/software/CBI/_rocky8/R-4.3.2-gcc10/lib64/R/bin/Rcmd check arkdb_0.0.16.tar.gz --no-manual -o /wynton/home/cbi/hb/repositories/future/revdep/checks/arkdb/old
 |   `-R --no-restore --no-echo --args nextArgarkdb_0.0.16.tar.gznextArg--no-manualnextArg-onextArg/wynton/home/cbi/hb/repositories/future/revdep/checks/arkdb/old
 |      |-sh -c LANGUAGE=en _R_CHECK_INTERNALS2_=1 '/wynton/home/cbi/shared/software/CBI/_rocky8/R-4.3.2-gcc10/lib64/R/bin/R' --vanilla --no-echo < '/scratch/hb/RtmpNYi9Br/file2a35234b2e24d0'
 |      |   `-R --vanilla --no-echo
 |      |      `-sh /wynton/home/cbi/shared/software/CBI/_rocky8/R-4.3.2-gcc10/lib64/R/bin/Rcmd BATCH --vanilla testthat.R testthat.Rout
 |      |          `-R -f testthat.R --restore --save --no-readline --vanilla
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             |-{R}
 |      |             `-{R}
 |      `-{R}

You probably know my rant by now, but this behavior is problematic when running on compute systems shared with others. I'm using more CPU resources than I'm allowed to/requested. I am also risking overusing the CPUs, e.g. running revdepcheck, I am actually ending up running two such instances at the same time.

Do you know where this N = all CPU cores parallelization comes from? I know that you depend on future.apply, but the Futureverse should respect the R CMD check --as-cran limitation, i.e. availableCores() returns at most 2 (two).

@cboettig
Copy link
Member

cboettig commented Dec 22, 2023 via email

@HenrikBengtsson
Copy link
Author

Hello.

Yes, one test does use future.apply to check parallel reads, but doesn’t set the number of cores.

It could be that testthat somehow circumvents R CMD check detection of parallelly::availableCores(); I created futureverse/parallelly#109 to investigate if this could be the case.

Some tests may be using duckdb now, which is threaded by default.

Ah, that could be it. From the htop screenshot, the child processes looks like forked processes (e.g. mclapply, "multicore"), but it could be that threads appear like that too. I'm quite sure it's not background R workers (e.g. PSOCK, "multisession"), because they'd have another set of command-line options.

Do you have a writeup on your rant?

Definitely; https://www.jottr.org/2022/12/05/avoid-detectcores/

You are familiar with ‘nice’ or containers / linux jails to control cpu use? Usually I find ram is the issue on shared
machines…

Yes; I can technically, protect others against this, and eventually our system will get cgroups2 protection others against this too. That would only protect others from overuse, but I would still overuse, resulting in lost of context switching, and more so the more CPU cores there are. And, in the bigger picture, this won't help anyone else out there. My blog post gives most of my arguments.

@cboettig
Copy link
Member

thanks, I agree with all your points that are specific to detectCores() -- as you see we're not setting that. (There are also reasons when a user might want to set this higher than the number of actual cores, e..g. when I/O is limiting). I'm less clear how your take applies to other tools, specifically lower-level libraries that are threaded by default (duckdb, arrow, gdal, openblas, loads of ml stuff etc). Most of these things are not impacted by the number of connections. I agree that connections in R can be a limited resource, but I don't quite understand the case that cpu use itself is limiting... anyway we are probably on the same page, I'm just a bit unclear how you feel about the default-threaded C-level applications like duckdb. To me, I think these are reasonable defaults that cause no real risk.

anyway, thanks for opening an issue and for all you do for the R community!

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

No branches or pull requests

2 participants