os: (cpus) pre-allocate the size of the result array

[gurgunday]

No description provided.

[juanarbol]

?

[gurgunday]

7 comes from the return type of getCPUs:

node/lib/os.js

Lines 150 to 157 in 2c09633

	model: data[i++],
	speed: data[i++],
	times: {
	user: data[i++],
	nice: data[i++],
	sys: data[i++],
	idle: data[i++],
	irq: data[i++],

[juanarbol]

Oh yeah, makes sense. Sorry.

[juanarbol]

Any particular reason of this patch?

[juanarbol]

Why not use the Primordial as the rest of the codebase?

[gurgunday]

Yeah so this is one of the smaller things I've seen in the codebase, sometimes we construct and push into arrays that we already know the size of

Pre-allocating the array with the size and filling it should be faster

[juanarbol]

Yeah, I'm sorry for being like this, would you mid writing a simple benchmark then? V8 is a bit weird sometimes, it does not like common sense sometimes.

[gurgunday]

Something like this?

function bench(size) {
    // Push method
    let start = performance.now();
    const arr1 = [];
    for (let i = 0; i < size; i++) {
        arr1.push(i);
    }
    console.log(`push: ${(performance.now() - start).toFixed(2)}ms`);

    // Pre-allocated array
    start = performance.now();
    const arr2 = new Array(size);
    for (let i = 0; i < size; i++) {
        arr2[i] = i;
    }
    console.log(`pre-allocated: ${(performance.now() - start).toFixed(2)}ms`);
}

// Run benchmark with 1 million elements
bench(10_000_000);

Chrome 133

VM83:8 push: 54.10ms
VM83:16 pre-allocated: 23.50ms

[gurgunday]

Or should I add a bench to Node.js?

[joyeecheung]

I think a benchmark that iterates over the returned CPU data and do something with it, say doing result &&= (cpus[i].speed > 2000), would be more realistic. Pre-allocating an array and making it holely can affect the performance of indexing due to change of element kinds.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.10%. Comparing base (cc7018e) to head (2c09633).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #57059   +/-   ##
=======================================
  Coverage   89.10%   89.10%           
=======================================
  Files         665      665           
  Lines      193203   193206    +3     
  Branches    37216    37216           
=======================================
+ Hits       172158   172164    +6     
- Misses      13768    13778   +10     
+ Partials     7277     7264   -13     

Files with missing lines	Coverage Δ
lib/os.js	97.33% <100.00%> (+0.01%)	⬆️

... and 36 files with indirect coverage changes

[gurgunday]

@anonrig PTAL, it should be an easy win in my opinion as the Array size is known in advance