src: improve node:os userInfo performance

[anonrig]

Improve node:os userInfo performance by reducing the cost of creating a v8::Object.

[codecov]

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.41%. Comparing base (bdc2662) to head (6d0bf5a).
Report is 37 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_os.cc	94.11%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55719      +/-   ##
==========================================
+ Coverage   88.38%   88.41%   +0.02%     
==========================================
  Files         654      654              
  Lines      187575   187759     +184     
  Branches    36096    36126      +30     
==========================================
+ Hits       165795   166012     +217     
+ Misses      15004    14996       -8     
+ Partials     6776     6751      -25     

Files with missing lines	Coverage Δ
lib/os.js	97.31% <ø> (-0.04%)	⬇️
src/node_os.cc	82.37% <94.11%> (+0.06%)	⬆️

... and 34 files with indirect coverage changes

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[lemire]

This has ben discussed and I don't want to block this... but this code might be too complicated. The following might suffice:

Suggested change

	static_cast<double>(static_cast<int32_t>(pwd.uid & 0xFFFFFFFF)));
	static_cast<double>(static_cast<int32_t>(pwd.uid)));

So that I understand... we start with this...

typedef struct uv_passwd_s {
    char* username;
    long uid;
    long gid;
    char* shell;
    char* homedir;
} uv_passwd_t;

(source: libuv documentation.)

node/deps/uv/include/uv.h

Lines 1212 to 1218 in a243225

	struct uv_passwd_s {
	char* username;
	unsigned long uid;
	unsigned long gid;
	char* shell;
	char* homedir;
	};

Under non-Windows systems.

But, for extra fun... under Windows, we have a different definition...

struct uv_passwd_s
{
  char *username;
  unsigned long uid;
  unsigned long gid;
  char *shell;
  char *homedir;
};

https://github.com/symplely/uv-ffi/blob/8e225f11537f9b34df7030652d00e1343d457ab1/headers/uv_windows.h#L2997C1-L3004C3

A long in Visual Studio is effectively an int32_t, it goes from -2,147,483,648 to 2,147,483,647. Whereas an unsigned long is an uint32_t.

Most other compiler systems will have long be either int32_t or int64_t (depending on whether we have a 32-bit or 64-bit system). They will have long be either uint32_t or uint64_t.

Now, Number::New's function signature takes a double. So the cast to double is mostly just to show what is happening (as it would happen implicitly).

So, setting aside the cast to double, we are doing...

uint32_t x =...
x = static_cast<int32_t>(x & 0xFFFFFFFF)

(If I got something wrong, please tell me.)

What is the purpose of the 0xFFFFFFFF?

[anonrig]

I missed these reviews. I'll open another pull-request to apply the changes.

[lemire]

@anonrig That's fine. I think your code is correct. It was just nitpicking.

[LiviaMedeiros]

The purpose of & 0xFFFFFFFF here was to mask higher bits. Unless my knowledge is horribly outdated, casting with overflow here would be UB (even though i doubt existence of implementations that don't just discard extra bits).

No strong opinion on casting to double, except for 'explicit > implicit' rule of thumb.

[lemire]

The purpose of & 0xFFFFFFFF here was to mask higher bits. Unless my knowledge is horribly outdated, casting with overflow here would be UB

An unsigned long under Visual Studio is a 32-bit unsigned integer. So we are casting an uint32 to an int32.

https://learn.microsoft.com/en-us/cpp/cpp/data-type-ranges?view=msvc-170

Node is C++20 and in C++20 casting from unsigned to signed and back is well defined.

No strong opinion on casting to double, except for 'explicit > implicit' rule of thumb.

I do not care at all. I only remarked that @anonrig's code was not perfectly consistent. I did not ask for the code to change.

[lemire]

Suggested change

	static_cast<double>(static_cast<int32_t>(pwd.gid & 0xFFFFFFFF)));
	static_cast<double>(static_cast<int32_t>(pwd.gid)));

[lemire]

The cast to double is implicit, but for Windows you make it explicit... if so then I would suggest making it explicit here:

Suggested change

	Local<Value> uid = Number::New(env->isolate(), pwd.uid);
	Local<Value> uid = Number::New(env->isolate(), static_cast<double>(pwd.uid));

[lemire]

Suggested change

	Local<Value> gid = Number::New(env->isolate(), pwd.gid);
	Local<Value> gid = Number::New(env->isolate(), static_cast<double>(pwd.gid));

[nodejs-github-bot]

Landed in 58a8eb4