src: optimize ALPN callback

[bnoordhuis]

It doesn't make sense from a performance perspective to retain an
arraybuffer with the ALPN byte string and look it up as a property on
the JS context object for every TLS handshake.

Store the byte string in the C++ TLSWrap object instead. That's both
a lot faster and a lot simpler.

---

First commit is minor cleanup for readability++.

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[tniessen]

It doesn't make sense from a performance perspective to retain an
arraybuffer with the ALPN byte string and look it up as a property on
the JS context object for every TLS handshake.

Store the byte string in the C++ TLSWrap object instead. That's both
a lot faster and a lot simpler.

Is this technically breaking in that users were previously able to modify the ArrayBuffer in-between handshakes?

[bnoordhuis]

I suppose, but it's an undocumented property on an undocumented property. Seems both very unlikely and not deserving of much sympathy.

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/44875
✔  Done loading data for nodejs/node/pull/44875
----------------------------------- PR info ------------------------------------
Title      src: optimize ALPN callback (#44875)
Author     Ben Noordhuis  (@bnoordhuis)
Branch     bnoordhuis:more-better-alpn -> nodejs:main
Labels     c++, lib / src
Commits    2
 - src: simplify ALPN code, remove indirection
 - src: optimize ALPN callback
Committers 1
 - Ben Noordhuis 
PR-URL: https://github.com/nodejs/node/pull/44875
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44875
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 03 Oct 2022 11:03:19 GMT
   ✔  Approvals: 1
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/44875#pullrequestreview-1136253893
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-10-10T22:25:49Z: https://ci.nodejs.org/job/node-test-pull-request/47165/
- Querying data for job/node-test-pull-request/47165/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   9926b89a99..19f3973828  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - 6ff429777a meta: move one or more collaborators to emeritus
 - 19f3973828 meta: move one or more collaborators to emeritus
--------------------------------------------------------------------------------
HEAD is now at 19f3973828 meta: move one or more collaborators to emeritus
   ✔  Reset to origin/main
- Downloading patch for 44875
From https://github.com/nodejs/node
 * branch                  refs/pull/44875/merge -> FETCH_HEAD
✔  Fetched commits as 19f397382810..0f5896716196
--------------------------------------------------------------------------------
Auto-merging src/crypto/crypto_common.cc
[main b6f6501ff8] src: simplify ALPN code, remove indirection
 Author: Ben Noordhuis 
 Date: Mon Oct 3 13:00:53 2022 +0200
 3 files changed, 4 insertions(+), 12 deletions(-)
Auto-merging src/env_properties.h
[main 821948a439] src: optimize ALPN callback
 Author: Ben Noordhuis 
 Date: Mon Oct 3 13:00:53 2022 +0200
 5 files changed, 19 insertions(+), 35 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: simplify ALPN code, remove indirection
PR-URL: #44875

Reviewed-By: James M Snell [email protected]
[detached HEAD 777dea3416] src: simplify ALPN code, remove indirection

Author: Ben Noordhuis [email protected]

Date: Mon Oct 3 13:00:53 2022 +0200

3 files changed, 4 insertions(+), 12 deletions(-)

Rebasing (3/4)

Rebasing (4/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

src: optimize ALPN callback
It doesn't make sense from a performance perspective to retain an

arraybuffer with the ALPN byte string and look it up as a property on

the JS context object for every TLS handshake.
Store the byte string in the C++ TLSWrap object instead. That's both

a lot faster and a lot simpler.
PR-URL: #44875

Reviewed-By: James M Snell [email protected]
[detached HEAD 3465634e85] src: optimize ALPN callback

Author: Ben Noordhuis [email protected]

Date: Mon Oct 3 13:00:53 2022 +0200

5 files changed, 19 insertions(+), 35 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/3284097315

[tniessen]

Agreed that the breaking change is unlikely to affect anything.

[nodejs-github-bot]

Landed in 2d0d997...fdadea8