cc @bmeurer @hashseed @mhdawson @codebytere

Note that building with this flag makes some of our addon tests fail:

Yeah, that’s expected. Enabling pointer compression changes the Node.js ABI, which is IIRC the reason why we haven’t done this so far.

Electron feels similarly - the memory savings are positive especially considering they're per-isolate but we're concerned about ABI compat of native modules

@gabrielschulhof can you confirm n-api can handle this case?

This needs a good story for our addon ABI

@addaleax my goal for this is to make it easier for folks to experiment with address compression. If you are ok, I'll add the option as "(experimental)" in the ./configure help.

@gabrielschulhof can you confirm n-api can handle this case?

N-API is unaffected.

This needs a good story for our addon ABI

@addaleax my goal for this is to make it easier for folks to experiment with address compression. If you are ok, I'll add the option as "(experimental)" in the ./configure help.

That would be good, but I’d prefer to explicitly call out that this mode does not currently support addons (at all)?

Definite +1 on marking this experimental and documenting the impact on native addons. Assuming those changes, this LGTM

That would be good, but I’d prefer to explicitly call out that this mode does not currently support addons (at all)?

Yeah iirc the plan was to consider having a build flag in V8 that ensures ABI stability regardless of whether pointer compression is enabled or not, by sacrificing some performance, since inline-implemented APIs would need to be replaced by non-inline versions.

Updated @jasnell @addaleax

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

I've removed the config from common.gypi. Please review.

cc @targos

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

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

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

I'm wondering whether there should be a warning printed at runtime when the node.js process starts.

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

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

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

@nodejs/build @nodejs/testing I would need some help here, as tests that seems unrelated are failing, however this should not change the default config.

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

@mcollina we currently rarely have green builds. There are lots of infrastructure failures and flaky tests. That's what seemed to have happened here as well.

CI: https://ci.nodejs.org/job/node-test-pull-request/27548/ ✔️

Landed in 086c7b4

@targos any concerns on backporting this to v12.x? I was testing it today, and if we add the following lines (pulled from dda658c) to common.gypi, it works:

diff --git a/common.gypi b/common.gypi
index 8c9076b735..856c6aa6ff 100644
--- a/common.gypi
+++ b/common.gypi
@@ -339,6 +339,12 @@
           }],
         ],
       }],
+      ['v8_enable_pointer_compression == 1', {
+        'defines': ['V8_COMPRESS_POINTERS'],
+      }],
+      ['v8_enable_pointer_compression == 1 or v8_enable_31bit_smis_on_64bit_arch == 1', {
+        'defines': ['V8_31BIT_SMIS_ON_64BIT_ARCH'],
+      }],
       ['OS == "win"', {
         'defines': [
           'WIN32',

The changes are non-intrusive, since it only impacts builds using --experimental-enable-pointer-compression. I can send a PR to backport it, unless there are other concerns.

where can i download a built version with --experimental-enable-pointer-compression enabled?

@tuananh I believe you have to build it yourself as it is not in any of the shipping binaries.

@targos any concerns on backporting this to v12.x? I was testing it today, and if we add the following lines (pulled from dda658c) to common.gypi, it works:

diff --git a/common.gypi b/common.gypi
index 8c9076b735..856c6aa6ff 100644
--- a/common.gypi
+++ b/common.gypi
@@ -339,6 +339,12 @@
           }],
         ],
       }],
+      ['v8_enable_pointer_compression == 1', {
+        'defines': ['V8_COMPRESS_POINTERS'],
+      }],
+      ['v8_enable_pointer_compression == 1 or v8_enable_31bit_smis_on_64bit_arch == 1', {
+        'defines': ['V8_31BIT_SMIS_ON_64BIT_ARCH'],
+      }],
       ['OS == "win"', {
         'defines': [
           'WIN32',

The changes are non-intrusive, since it only impacts builds using --experimental-enable-pointer-compression. I can send a PR to backport it, unless there are other concerns.

If this does get backported it will need to land with #33688 to fix building addons.

Probably not worth it. We have pointer compressions builds for v14 now, and the pointer compression implementation on v12 has bugs as well as performance issues.

Why wouldn’t this be useful now that we’re on node 18?