net: replace brand checks with identity checks

[anonrig]

An identity check is always faster than a brand check.

[nodejs-github-bot]

Review requested:

• @nodejs/net

[nodejs-github-bot]

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

[ljharb]

primitives don't have identity, and typeof isn't checking a brand - but terms aside, i'm very surprised that x === undefined is faster than the many-decades-optimized typeof x === 'undefined'. do you have benchmark data that demonstrates that?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.19%. Comparing base (8342183) to head (7bc71d9).
Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57341      +/-   ##
==========================================
- Coverage   90.19%   90.19%   -0.01%     
==========================================
  Files         630      630              
  Lines      185195   185195              
  Branches    36249    36243       -6     
==========================================
- Hits       167036   167033       -3     
- Misses      11132    11140       +8     
+ Partials     7027     7022       -5     

Files with missing lines	Coverage Δ
lib/net.js	94.95% <100.00%> (ø)

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

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

[anonrig]

primitives don't have identity, and typeof isn't checking a brand - but terms aside, i'm very surprised that x === undefined is faster than the many-decades-optimized typeof x === 'undefined'. do you have benchmark data that demonstrates that?

I don't have any benchmark results to share at the moment. Even if I had, these changes will not move the needle a tiny single bit.

[ljharb]

… then why make the change if it won't move the needle?

[anonrig]

… then why make the change if it won't move the needle?

For refactor purposes, making the code align with the rest of the codebase, also for readability.

But these are all open to discussion and can change from people to people. So, it's within your right to block if you think this is unnecessary.

No hard feelings :-)

[ljharb]

Oh no, there's absolutely no reason to block here :-) if it's the consistent style in the project then that's sufficient reason on its own. I was just asking the question because if there's any performance-motivated change without a benchmark, then it's almost a certainty that the performance will be regressed in the future, so if it's worth making it faster, it's presumably worth keeping it faster.

[nodejs-github-bot]

Landed in 3a497dc