feat: napi rebuild - #6660
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #6660 +/- ##
============================================
+ Coverage 61.64% 61.82% +0.18%
============================================
Files 556 556
Lines 58782 59073 +291
Branches 1886 1898 +12
============================================
+ Hits 36236 36524 +288
- Misses 22505 22506 +1
- Partials 41 43 +2 |
Performance Report✔️ no performance regression detected Full benchmark results
|
7fb847a to
8dda2ff
Compare
| } from "./jobItem.js"; | ||
| import {asyncVerifyManySignatureSets} from "./verifyManySignatureSets.js"; | ||
|
|
||
| // defaultPoolSize should return core count - 1 to keep main thread on core. We |
There was a problem hiding this comment.
I am wondering if we should always go with logical CPU count - 1 or have a max cap at some point, e.g. does it make sense to have pool size of 64 or even 128?
Same goes for other worker pools we use
There was a problem hiding this comment.
I have it sized at number of cores. There is a fair amount of idle time on the main and network thread so the kernel is switching those out for bls in down times.
I need to update that note.
I tried with cores - 2 at first to keep main and network thread on at all times but there was performance degradation. Tried cores -1 also but found the better but similar. Best results were sizing the pool at the number of cores and letting the kernel do the management. Can fine tune further though I think
There was a problem hiding this comment.
But regarding my question, is there an upper bound to the pool where it just does not make sense to spawn more workers?
I tried with cores - 2 at first to keep main and network thread on at all times but there was performance degradation.
I am surprised this makes a difference, because we kinda assume here that Lodestar is the only process running on the server which for most setups is not the case.
There was a problem hiding this comment.
I would assume more than the number of cores is generally bad. So as an upper limit I tried to stick with number of cores.
The bls work is very math heavy and the assembly portions of the code (synchronous functions) stay on thread for most of the execution time. Attempting to do more parallel calls than there are cores will net a reduction of overall performance because of the time swapping thread context (moving active thread to wait state while another thread is being executed by a core). Similar to how we are moving the main thread and worker thread to wait while bls runs. The kernel knows when a process is idle though and optimizes for this so that processes that are active get run and ones that are idle can wait for other threads to run.
There are some other nuances to that though. Multi-threading allows a core to have more than one "active" thread that can execute at any give time although this is not a standardized thing. And how/when the thread is "active" also is non-standard. Each manf. has a different core/thread ratio and different way of handling what is actually running assembly at a given time. I would guess that we can tune the number of threads up slightly but how much will be empirical, and results will be specific to the machine they were tested with.
As for the part that the EL is also running on the box. This is a good point. My evidence for setting the threadpool size was empirical. Ran the same branch with a few different values on separate feature groups. And was rough at best but the results were pretty obvious at the time however they did not get persisted anywhere other that me going "cores = threads is better" and I went with it. There was a lot of variables floating around at the time so i just simply went with what looked best. At this point, now that we have narrowed a lot down and have a much more stable set of factors it will be a good idea to do a more methodical approach to setting this value.
Also will be a good idea to include EL performance in the calculation for setting. Not sure how to rate that side's efficiency but should be easy enough with a bit of digging. Will def add this to the to-do list
There was a problem hiding this comment.
@nflaig as an example I found on OSX that cores = threads (ie 10 on my M1) but on our hzax41 boxes the threads = 2 x cores. Generally when requesting the "number of cores" or "how much multithreading" is available the number of logical cores is what is reported.
Linux - Cloud VM
> user@mainnet-hzax41:~$ awk -F': ' '/cpu cores/{print $2;exit}' /proc/cpuinfo
6
> user@mainnet-hzax41:~$ grep -c ^processor /proc/cpuinfo
12console.log(require(`os`).availableParallelism())
// > 12OSX - M1 (some irrelevant lines removed)
❯ sysctl -a | grep cpu
hw.ncpu: 10
hw.activecpu: 10
hw.physicalcpu: 10
hw.physicalcpu_max: 10
hw.logicalcpu: 10
hw.logicalcpu_max: 10
machdep.cpu.cores_per_package: 10
machdep.cpu.core_count: 10
machdep.cpu.logical_per_package: 10
machdep.cpu.thread_count: 10
machdep.cpu.brand_string: Apple M1 Maxconsole.log(require(`os`).availableParallelism())
// > 10There was a problem hiding this comment.
@nflaig as an example I found on OSX that cores = threads (ie 10 on my M1) but on our hzax41 boxes the threads = 2 x cores. Generally when requesting the "number of cores" or "how much multithreading" is available the number of logical cores is what is reported.
Also be aware of docker, often times I saw logical CPUs to equal physical cores there.
There was a problem hiding this comment.
I just noticed that at least for worker_threads, it's a noticeable overhead at startup, see issue during tests #5608 but I am not sue this matters. That's why I was asking because you have a better idea about this then I do
worker_thread != kernel thread though.... for a worker thread a whole JS environment is spun up and that does take a considerable amount of time. kernel threads are not the same... the time is not 0 but its very very very low (likely milliseconds). Not sure how long though but I can tell you that starting this branch with libuv it goes to the next step almost immediately (not sure exactly where in the flow the workers are started though in practice. it could be even before the bls class gets instantiated when fs happens or it could be when the first bls work gets queued during sync but not sure honestly) and starting with workers there is maybe a 10-15 second delay to start all the nodejs worker_threads
Found this on SO:
https://stackoverflow.com/questions/18274217/how-long-does-thread-creation-and-termination-take-under-windows
There was a problem hiding this comment.
@nflaig as an example I found on OSX that cores = threads (ie 10 on my M1) but on our hzax41 boxes the threads = 2 x cores. Generally when requesting the "number of cores" or "how much multithreading" is available the number of logical cores is what is reported.
Also be aware of docker, often times I saw logical CPUs to equal physical cores there.
I would guess, but need to confirm 100%, that docker would read the logical cpu count and report that to the container as the physical and logical count. The docker runtime has a setting to throttle how many "CPU's" are available to the daemon. I would assume they are using the soft/logical limit for that similar to how all software consumes the underlying system resources.
There was a problem hiding this comment.
I would guess, but need to confirm 100%, that docker would read the logical cpu count and report that to the container as the physical and logical count.
That was something I noticed when working on system metrics
lodestar/packages/beacon-node/src/monitoring/system.ts
Lines 63 to 67 in 2dae605
Looking at the metrics from my server, it seems to be still the case
There was a problem hiding this comment.
worker_thread != kernel thread though
yeah, that's what I thought as well, it might not matter for libuv threads but we should probably consider an upper bound for any other worker pools, although might not be relevant if we anyhow replace the BLS worker implementation, and it's not that problematic for keystore decryption as those are limited by number of keys already and will be removed once decryption is completed.
|
latest profile show a lot of time used for timer 
I think that's also the reason why feat2_1k_main_thread_2024-04-16T01:34:42.576Z.cpuprofile.zip we really need to stop |
|
closed in favor of #6616 |
3 participants
Description
Uses the latest published versions of
chainsafe/bls@8.0.0andchainsafe/blst@1.0.0.Includes PR comments moved from #6616
Base of PR was branch
mkeil/test-blst-7from #6362 and has the added verification randomness for same messages added. Retains workers and herumi still.Currently deployed to
feat1for metrics collection