node
node copied to clipboard
Add runtime information about libc version
This topic already been discussed before in the following issues/PRs:
- https://github.com/nodejs/node/issues/39877
- https://github.com/nodejs/node/pull/41338
While this was considered a resolved topic as we can get the information from process.report.getReport
, I want to put more focus on the performance implications of not exposing this information.
Current Performance
Below, is the op/s to check if the system is musl
using many methods I saw using this code search:
process.report.getReport() x 218 ops/sec ±4.16% (76 runs sampled)
isMusl x 167 ops/sec ±1.94% (74 runs sampled)
isMuslByFile x 95,904 ops/sec ±2.67% (80 runs sampled)
Fastest is isMuslByFile
benchmark.js
const Benchmark = require('benchmark');
const suite = new Benchmark.Suite();
const decoder = new TextDecoder();
const isMusl = () => {
try {
const lddPath = decoder.decode(require('child_process').execSync('ldd --version')).includes('musl')
return lddPath
} catch (e) {
return true
};
}
function isMuslByFile() {
try {
return readFileSync('/usr/bin/ldd', 'utf8').includes('musl')
} catch (e) {
return true
}
}
suite.add('process.report.getReport()', function () {
const id = !process.report.getReport().header.glibcVersionRuntime;
});
suite.add(`isMusl`, function () {
const result = isMusl();
});
suite.add(`isMuslByFile`, function () {
const result = isMuslByFile();
});
suite
// add listeners
.on('cycle', function (event) {
console.log(String(event.target));
})
.on('complete', function () {
console.log('Fastest is ' + this.filter('fastest').map('name'));
})
.run({
async: true,
});
Above, is just the time spent in a benchmark, in real life, v8 will not optimize this function soo much, below the profile of swcx (rust binding for swc
).
Command: node --cpu-prof swcx/index.js Profile: swcx.cpuprofile.zip
Implications
These libraries usually implement their own version instead of installing a library like detect-libc to have a standardized way to check if the system is musl
. This means that almost every nodejs
package that checks if it is musl
will increase the startup time by some range of 5~30ms.
~But even using detect-libc
, it didn't cache the result value if the system is musl
, so the problem with initialization is also a problem here, and this library is downloaded 9 million times, which means a lot of time can be saved here.~
Actually, detect-libc
caches the value after the first run, so if multiple libraries use this lib to check for musl
, they can benefit from this cache.
This issue can also affect NPM
or any package manager if they decide to follow this RFC: https://github.com/npm/rfcs/pull/519 if they decide to provide support for https://github.com/npm/rfcs/pull/519#discussion_r831795705. With this implementation, possibly every x64 linux will have some startup penalty but at least this will only be once, as the library no longer needs to do this check.
Conclusion
There is a lot of time that can be saved by just exposing that information directly instead of relying on process.report.getReport
. The main benefit will be for many libraries that expose native bindings with NodeJS.
So, I want to raise some feedback about this, especially from @styfle, @Brooooooklyn, @richardlau and @bnoordhuis.
I want to put more focus on the performance implications of not exposing this information.
Why? Have you actually found this to be a major bottleneck somehow?
I want to put more focus on the performance implications of not exposing this information.
Why? Have you actually found this to be a major bottleneck somehow?
Yes, see https://github.com/nodejs/node/issues/46060.
Yes, see #46060.
So then this issue is a duplicate of https://github.com/nodejs/node/issues/46060?
Why? Have you actually found this to be a major bottleneck somehow?
I don't know what you mean by major bottleneck but spending more than 5ms just to check if the system is musl
when the information can be static sounds to me a thing that we can optimize.
So then this issue is a duplicate of https://github.com/nodejs/node/issues/46060?
No, my intention is not to try to make getReport
faster, is just to expose the information of libc
to be used by any rust library.
getReport
is way more difficult to optimize and due to the nature of JS<->C++ serialization.
Also, napi-rs also checks musl when importing/running your code, this is the default emitted code from Napi, so improving this helps a lot NodeJS libraries with Rust bindings.
I don't know what you mean by major bottleneck but spending more than 5ms just to check if the system is
musl
when the information can be static sounds to me a thing that we can optimize.
If the scenario we're discussing here is downloading of precompiled binaries for node addons, I think the binary download, the related fs operations, and other parts of the process are going to dwarf calls to getReport()
(once the previously mentioned DNS-related issue has a workaround), so even going from something like 500ms to 5ms is not going to be noticeable, especially for a task (installation of packages) that happens once.
Also, napi-rs also checks musl when importing/running your code, this is the default emitted code from Napi, so improving this helps a lot NodeJS libraries with Rust bindings.
Again, improving getReport()
should make this a non-issue. Additionally since napi-rs
contains a fallback, perhaps it should just use that all the time if that is deemed to be faster?
so even going from something like 500ms to 5ms is not going to be noticeable, especially for a task (installation of packages) that happens once.
It depends on the implementation but I agree that this can't be a big issue if it only happens on installation, but it will depend on whether NPM accepts and adds support for musl
in that RFC, which I don't know if or when it will happen.
Additionally since napi-rs contains a fallback, perhaps it should just use that all the time if that is deemed to be faster?
I'm discussing this topic on https://github.com/lovell/detect-libc/issues/18#issuecomment-1565572368 to understand better the implications, if it's okay to use always the fallback, for sure that should be used all the time.
Anyway, 96k op/s is fine, but it could just be waay faster
If the scenario we're discussing here is downloading of precompiled binaries for node addons, I think the binary download, the related fs operations, and other parts of the process are going to dwarf calls to getReport() (once the previously mentioned DNS-related issue has a workaround), so even going from something like 500ms to 5ms is not going to be noticeable, especially for a task (installation of packages) that happens once.
This doesn't necessarily happen only when downloading, it's also typically necessary to check the libc type each time before loading the addon.
Only resolving it once when downloading and writing the addon file under a common name is often not practical (especially when distributing the addon binaries via npm; although in our case it's not possible for certain reasons even though the download logic is custom and the binaries are downloaded from S3).
To clarify, in my case we're not currently using process.report.getReport()
to get libc information. However, I wanted to provide some additional context to this discussion, as this seems relevant.
This isn't perfectly solvable because there is no reliable way to feature-detect musl libc.
The approach that process.report.getReport()
takes (check the system's dynamic linker) is... let's be diplomatic and call it "best effort" (read: flawed, can produce both false positives and false negatives.)
You could construe the absence of glibc-specific defines or symbols as an indicator of musl but obviously that's not foolproof, glibc and musl aren't the only linux libcs.
Opening /proc/self/exe and scanning the ELF header for the dynamic linker section or shared objects section won't work when node is statically linked.
Basically, every approach gets it right some of the time but never all the time.
This isn't perfectly solvable because there is no reliable way to feature-detect musl libc.
The approach that process.report.getReport() takes (check the system's dynamic linker) is... let's be diplomatic and call it "best effort" (read: flawed, can produce both false positives and false negatives.)
Good to know, I just assumed this was solvable at compile time, and without checking the code I also assumed that what getReport
was doing was more reliable than the current userland approaches. Thanks so much for correcting these assumptions.
Opening /proc/self/exe and scanning the ELF header for the dynamic linker section or shared objects section won't work when node is statically linked.
I feel like this might actually be the best solution in the context of loading native addons. If node is statically linked, then libc doesn't matter since dynamic loading is not supported anyway. But then again, it's possible to do this in userland and it doesn't necessarily have to be in core.
If node is statically linked, then libc doesn't matter since dynamic loading is not supported anyway.
That's true for musl but it sort of works with glibc (if you have libc.so around and don't need multi-namespace support.)
I see, thanks for clarification.
Currently, all the NodeJS (>10.x) applications that check for musl, use the getReport
.
The approach that process.report.getReport() takes (check the system's dynamic linker) is... let's be diplomatic and call it "best effort" (read: flawed, can produce both false positives and false negatives.)
And even with these possible issues, they still use this method a lot and it's very reliable as no lib maintainer has opened an issue to complain that it's impossible to use musl
and NodeJS.
So the question is, to introduce a new API in NodeJS, how accurate should it be?
And even though I opened this thread to discuss exposing the libc
version, my main goal is to have a quick way to detect musl
, so if we limit our implementation to just exposing that information, I think everyone will be satisfied.
Also, I opened a PR to improve the detection on detect-libc
to detect via /usr/bin/ldd
.
I'm not an expert in this field so I don't know exactly how much accurate this solution will be but I think it could be more reliable than getReport
since we check directly in the linker and much faster (30ms to 0.16ms).
If we could do this same method directly C++ side, it could be even faster and we can fallback to getReport
when we don't have access to that file because of the permission model.
Finally, I'll try to open a PR on NodeJS to discuss the possible implementation of musl
after my PR on detect-libc
, so the discussion will be clearer as we will have some code to read/execute.
So the question is, to introduce a new API in NodeJS, how accurate should it be?
To ask the question is to answer it, isn't it? :-) It can't be half-baked.
To ask the question is to answer it, isn't it? :-) It can't be half-baked.
If we agree that the current way of using getReport
is accurate enough, we just need to reopen https://github.com/nodejs/node/pull/41338
If we want a more precise way we can look at /usr/bin/ldd
like I'm doing in detect-libc
, in this case, I can open a PR to do that.
What do you think could be the best strategy here?
Neither, for the reasons outlined above.
So, let's separate into two main problems:
Expose libc version
Those who want to get the information about the libc and the libraries that want to check if musl is supported can also check this information.
It will not cover all the edge cases of musl
but at least will speed up detect-libc
, which is downloaded 10M week.
About the implementation, according to https://github.com/nodejs/node/pull/41338#pullrequestreview-840421034, maybe process.versions
is not the best place, but it can be inside process
.
Expose function to check for musl
Maybe we can export this function from os
, about the implementation, we have some strategies:
- Read the file content of
/usr/bin/ldd
and check for the presence ofmusl
, which will be the first option ofdetect-libc
. (ref) - Use
glibcVersionRuntime
fromgetReport
, the first option ofdetect-libc
. (ref) - Use
sharedObjects
fromgetReport
, the second option ofdetect-libc
. (ref) - Run
ldd --version
and check for the presence ofmusl
, the last option ofdetect-libc
. (ref)
The most reliable way of checking should be sharedObjects
, which accords to the musl faq:
Q: Where is ldd? musl’s dynamic linker comes with ldd functionality built in. Just create a symlink from ld-musl-$ARCH.so to /bin/ldd. If the dynamic linker was started as “ldd”, it will detect that and print the appropriate DSO information.
Both problems are solved in user space but with slow down on startup, and while we can speed up the main library (detect-libc
), many other libraries/applications have their own implementation (or they can't install detect-libc
, eg: exports by napi-rs), so having a standardized solution in the NodeJS core might be more useful to the ecosystem than having multiple implementations.
I think you're trying to argue that approach X is less wrong than Y. Maybe, but it's irrelevant - they're still both wrong. It's like saying you're less pregnant now.
"But performance" is not a credible argument either. It's not a performance-sensitive thing. Even if it was, well hurray, you give the wrong answer faster now.
The premise is simply flawed. It's no use opening pull requests because they will only get rejected.
Since we couldn't come to a consensus on what approach we should take for this, I'll close this issue as I'm satisfied with solving this problem in userland with the current strategy of detecting musl
using /usr/bin/ldd
.