node-api: remove RefBase and CallbackWrapper

[vmoroz]

This PR simplifies internal Node-API implementation to reduce the code complexity and to reduce allocated memory size. There are no API changes or changes to the code behavior. All existing tests are passing and no new tests are needed.

• Removed RefBase class.
  • Reference class directly inherits from RefTracker.
  • napi_set_instance_data and napi_get_instance_data use TrackedFinalizer instead of RefBase.
• v8impl::Ownership enum is renamed to v8impl::ReferenceOwnership to better reflect its purpose.
  • v8impl::ReferenceOwnership size changed to one byte to reduce Reference size.
• Finalizer class became non-virtual to reduce its size.
  • All Finalizer fields became private for better encapsulation.
  • TrackedFinalizer and TrackedStringResource size changed from 64 to 56 bytes on 64-bit platforms.
• Reference class is split up into three classes depending on the usage scenario: Reference, ReferenceWithData, and ReferenceWithFinalizer.
  • Previously Reference instance size on 64-bit platforms was 88 bytes.
  • New Reference size is 40 bytes, ReferenceWithData is 48 bytes, and ReferenceWithFinalizer is 72 bytes.
  • ReferenceWithFinalizer size reduced by 16 bytes due to changes to Finalizer (8 bytes) and removal of RefBase that along with the v8impl::ReferenceOwnership to be one byte gained reduction of another 8 bytes.
  • Instances of Reference and ReferenceWithData are not queued for finalization since they do not have user finalizer callbacks. They are finalized immediately.
• Simplified FunctionCallbackWrapper.
  • Removed base classes CallbackWrapper and CallbackWrapperBase
  • All methods became non-virtual.
  • It should slightly improve perf for each function call.

[nodejs-github-bot]

Review requested:

• [ ] @nodejs/node-api

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/53590
✔  Done loading data for nodejs/node/pull/53590
----------------------------------- PR info ------------------------------------
Title      node-api: remove RefBase and CallbackWrapper (#53590)
Author     Vladimir Morozov <[email protected]> (@vmoroz)
Branch     vmoroz:PR/remove_refbase -> nodejs:main
Labels     c++, node-api, author ready, needs-ci
Commits    4
 - node-api: remove RefBase and CallbackWrapper
 - reduce size of Reference
 - address PR feedback
 - add const for refcount()
Committers 2
 - Vladimir Morozov <[email protected]>
 - GitHub <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/53590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - node-api: remove RefBase and CallbackWrapper
   ⚠  - reduce size of Reference
   ⚠  - address PR feedback
   ⚠  - add const for refcount()
   ℹ  This PR was created on Tue, 25 Jun 2024 21:35:58 GMT
   ✔  Approvals: 5
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/53590#pullrequestreview-2141773353
   ✔  - Gabriel Schulhof (@gabrielschulhof): https://github.com/nodejs/node/pull/53590#pullrequestreview-2148413989
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/53590#pullrequestreview-2151082401
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/53590#pullrequestreview-2157911770
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/53590#pullrequestreview-2175228053
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-08-23T13:20:52Z: https://ci.nodejs.org/job/node-test-pull-request/61377/
- Querying data for job/node-test-pull-request/61377/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

[vmoroz]

@nodejs/node-api , could you please sign off the latest changes. The last CI build is succeeded and we must be able to merge the PR.

[vmoroz]

@mhdawson , could you please sign off the latest change? The last change was only adding the const keyword to refcount() suggested by @anonrig. The PR passed all the CI validation and is ready to merge.

[nodejs-github-bot]

Landed in 431ac161e65698152de197703fb30c89da2b6686

[vmoroz]

@legendecas , thank you for merging it! :)