tweak how memory of worker effect number of parallel jobs#142
tweak how memory of worker effect number of parallel jobs#142KristofferC wants to merge 1 commit into
Conversation
| function default_njobs(; cpu_threads = Sys.CPU_THREADS, free_memory = available_memory()) | ||
| jobs = cpu_threads | ||
| memory_jobs = Int64(free_memory) ÷ (2 * Int64(2)^30) | ||
| return max(1, min(jobs, memory_jobs)) | ||
| function default_njobs(; cpu_threads = Sys.CPU_THREADS, total_memory = Sys.total_memory(), | ||
| memory_per_worker = DEFAULT_MEMORY_PER_WORKER) | ||
| memory_jobs = Int64(total_memory) ÷ Int64(memory_per_worker) | ||
| return max(1, min(cpu_threads, memory_jobs)) |
There was a problem hiding this comment.
Why using total memory instead of the actually available one? This would oversubscribe (read: crash very badly) busy machines.
There was a problem hiding this comment.
Why using total memory instead of the actually available one?
Many many reasons:
- Free memory has nothing really to do with how much memory you can actually use. Most computers will cache up most of the RAM by just using the computer for a while (why not, the memory is there to be used), but if an application needs more memory, it drops some of that cache.
- You only do the free memory sample at the start, if the machine is actually busy, this snapshot is unlikely to mean much during the course of the testing.
- Using free memory is not deterministic, you hit some memory spike at the point where you do the free memory sample and all of a sudden your CI takes 30 times longer
- It is arguably better to crash loudly than to have a CI run silently extremely slowly. At that point, you know you have to set some explicit options (which you already have to do if your workers are very memory heavy).
- If you are on a very large shared machine and want to run heavily parallel tests, you probably should use some queuing system (slurm) instead of just blasting it on the login node.
- For a package whose purpose is to make your tests go faster by using the power of your machine, optimizing for the uncommon case of running tests on a very strong machine that you apparently can not run heavy stuff on is the wrong default in my opinion. You probably have to set explicit options there anyway.
There was a problem hiding this comment.
It is arguably better to crash loudly than to have a CI run silently extremely slowly.
Hard disagree on this, because the premise is false. An OOM on Linux isn't a loud crash, it's a job hanging without explanation until it's cancelled because it hits a timeout. Doesn't seem like a great outcome to me.
There was a problem hiding this comment.
So run with 1 worker always by default then if the goal is to maximize the probability of not running out of memory. Anything else can always be argued in the way you are doing now. It's like saying julia -tauto should somehow guess how much memory each thread will use (based on some anecdotal example), look at free memory and do this type of calculation and give you 1 thread today and 5 threads tomorrow etc.
|
I think I'm overall ok with this, but prefer checking available memory rather than the total one, as mentioned above. |
|
I disagree about free memory (#142 (comment)), it's not really a useful thing to look at, especially not as a snap-shot. As a comparison, the Julia GC looks at total memory if it needs a memory "budget" and handles problems with available memory as it is running. Here is my free memory on my mac that doesn't really do anything: Using this 50MB number for how much test this machine can run is not really a good idea. |
|
As of #29, we use a more method that more representative of the amount of memory that a mac has available to use than I don’t know about higher-memory devices, but the 7 and 8 GB mac that are in guthub and buildkite CI would be hitting (#124) by default if job selection were based on total memory. |
|
Again, the free memory is not useful because the machine can for swap out old memory that is not needed right now to make room for other memory.
Look at #73 (comment). The difference between using the defaults here vs enabling all cores is 12m vs 7m. Everywhere I use this package, I have to add https://github.com/KristofferC/NearestNeighbors.jl/blob/e56957a3e5941739cddfb487146e4c8c034fa6df/test/runtests.jl#L9 to not make the CI workers horribly slow compared to what they could be. And if you have very big processes, you will just have to manually reduce the number of workers, that's completely normal. But everyone shouldn't suffer because of that there exists a package that cannot run with the defaults. The goal should be fast by default. |
3 participants
Based on the discussion in #73 this PR does the following changes:
default_njobsnow computesmin(cpu_threads, total_memory ÷ memory_per_worker)— it uses total system memory instead of free memory (which measured whatever else happened to be running and made the default non-deterministic), and the per-worker estimate dropped from 2 GiB to 1 GiB (newDEFAULT_MEMORY_PER_WORKERconstant).memory_per_workerkwarg on runtests, threaded through todefault_njobs, so memory-heavy packages (CUDA.jl, Enzyme.jl) can declare e.g. 3 GiB per worker and get a conservative default without taxing everyone else. Ignored when jobs are set explicitly.PARALLELTESTRUNNER_NUM_JOBSenv var (name from PR Define environment variable for overriding number of jobs #65) as a CI-friendly override, with precedence--jobs=N> env var > computed default.JULIA_CPU_THREADS(which didn't work when the memory clamp was binding) to mention the env var; updated--helptext, README, and docstrings; rewrote/extended the tests.This has the following effect on the machines in #73:
This seems like much more reasonable defaults to me. Using 3 cores by default on M4 is just not ok :P
(Claude Fable helped prepare this PR)