os: improve performance of hostname and homedir

[Uzlopak]

This PR improves the performance of the homedir and hostname.

 confidence improvement accuracy (*) (**) (***)
os/hostname.js n=1000000 *** 37.28 % ±1.16% ±1.54% ±2.01%
Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 1 comparisons, you can thus expect the following amount of false-positive results:
0.05 false positives, when considering a 5% risk acceptance (*, **, ***),
0.01 false positives, when considering a 1% risk acceptance (**, ***),
0.00 false positives, when considering a 0.1% risk acceptance (***)

 confidence improvement accuracy (*) (**) (***)
os/homedir.js n=1000000 *** 60.71 % ±4.49% ±6.00% ±7.88%
Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 1 comparisons, you can thus expect the following amount of false-positive results:
0.05 false positives, when considering a 5% risk acceptance (*, **, ***),
0.01 false positives, when considering a 1% risk acceptance (**, ***),
0.00 false positives, when considering a 0.1% risk acceptance (***)

 confidence improvement accuracy (*) (**) (***)
os/uptime.js n=100000 *** 0.69 % ±0.19% ±0.26% ±0.33%
Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 1 comparisons, you can thus expect the following amount of false-positive results:
0.05 false positives, when considering a 5% risk acceptance (*, **, ***),
0.01 false positives, when considering a 1% risk acceptance (**, ***),
0.00 false positives, when considering a 0.1% risk acceptance (***)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54525/

[Uzlopak]

@H4ad
@mscdex

Removed the caching.

[Uzlopak]

Updated the benchmark in initial post.

We should discuss the caching. But this is the minimal consensus, which should be mergable.

[H4ad]

@Uzlopak do we have an idea how much time it takes in ms to get the homedir?

If it's quick, I think we can land without a cache, the improvements are good enough.

If it's slow, I think adding caching will not be bad, accords to docs: https://docs.libuv.org/en/v1.x/misc.html#c.uv_os_homedir doesn't look like this information will change during the execution of the process. We only need to worry about the snapshot.

[Uzlopak]

I dont know how to measure the lookup time

[H4ad]

Code:

const{homedir, hostname}=require('os')letnow=performance.now();letvalue=homedir();console.log(`Diff: ${performance.now()-now}ms`);now=performance.now();value=hostname();console.log(`Diff: ${performance.now()-now}ms`);

Output:

Diff: 0.045215003192424774ms
Diff: 0.01658099889755249ms

Since we don't have any hot path calling this code multiple times, I think we can skip caching to not add complexity without a good reason.

[Uzlopak]

ah, you mean like that.

Maybe programms like npm or pnpm need homedir more excessively.

[mscdex]

Package managers are more likely to have I/O or compression/decompression be their bottleneck rather than something like os.homedir().

[Uzlopak]

@anonrig can you please add the author-ready tag?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54541/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54562/

[Uzlopak]

@mscdex
If #49990 lands i want to anyway split to avoid an additional hideStackFrames call.

[Uzlopak]

@mcollina can i interest you into reviewing this PR?

[Uzlopak]

@anonrig
What about now?

[anonrig]

@anonrig

What about now?

The CI is locked. It seems the errors are fixed in main. Can you rebase and force push?

[Uzlopak]

@anonrig
done

[Uzlopak]

@anonrig
can it be merged?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54776/

[Uzlopak]

@anonrig can you give the CI some love please? :D

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54797/

[Uzlopak]

@anonrig
Seems that the checks pass?

[nodejs-github-bot]

Landed in 0f0dd1a

[maanu1234]

👍

[maanu1234]

👍