Perf of ReflectConstruct

[H4ad]

I think this is the second PR I saw removing the usage of ReflectConstruct: nodejs/node#49546

The first one improved a lot the creation of WebStreams, and there are much more cases where this function is used, so I think that it worth exploring the other usages to see if possible to remove that function.

[mcollina]

In the past @aduh95 documented which primordials created a performance issue, I think we should add this there too.

[aduh95]

It's in the primordials.md document: https://github.com/nodejs/node/blob/HEAD/doc/contributing/primordials.md

[H4ad]

Based on nodejs/node#49730, we can improve the performance of structuredClone up to 50% (at least on my internal benchs) if we could remove ReflectConstruct.

This could be beneficial for webstreams and blobs, but I was not able to make it work, I tried:

function InternalClonedBlob() {
  markTransferMode(this, true, false);
}

ObjectSetPrototypeOf(InternalClonedBlob.prototype, Blob.prototype);
ObjectSetPrototypeOf(InternalClonedBlob, Blob);

InternalClonedBlob.prototype[kDeserialize] = () => {};

function ClonedBlob() {
  var cloned = new InternalClonedBlob();

  cloned.constructor = Blob;

  return cloned;
}

Which throws:

node:internal/worker/js_transferable:38
      throw new Error(`Unknown deserialize spec ${deserializeInfo}`);
      ^

Error: Unknown deserialize spec internal/blob:ClonedBlob
    at node:internal/worker/js_transferable:38:13
    at receiveMessageOnPort (node:internal/worker/io:410:19)
    at structuredClone (node:internal/structured_clone:24:10)
    at Object.<anonymous> (/home/h4ad/Projects/opensource/node-copy-3/test-khafra.js:3:13)
    at Module._compile (node:internal/modules/cjs/loader:1429:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1487:10)
    at Module.load (node:internal/modules/cjs/loader:1261:32)
    at Module._load (node:internal/modules/cjs/loader:1077:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:99:12)
    at node:internal/main/run_main_module:23:47

I was able to make a version that works doing:

class ClonedBlob extends Blob {
  static {
    markTransferMode(this, true, false);
    this[kDeserialize] = () => {};
  }
}

ClonedBlob.prototype.constructor = Blob;

It prints correctly the results of:

const { Blob } = require('buffer');

(async () => {
  console.log(structuredClone(new Blob(['hello'])));
  console.log(await structuredClone(new Blob(['hello'])).text());
  console.log(structuredClone(new Blob([])).constructor);
  console.log(structuredClone(new Blob([])).constructor === Blob);

  console.log(new Blob(['a']).slice(0, 1).constructor);
})();

Blob { size: 5, type: '' }
hello
[class Blob]
true
[class Blob]

But when we run the benchmark, it throws an error after a couple executions:

[00:00:11|%   8| 0/1 files |  5/60 runs | 0/1 configs]: blob/clone.js node:internal/worker/io:410
  const message = receiveMessageOnPort_(port?.[kHandle] ?? port);
                  ^

Error: Unable to deserialize cloned data.
    at receiveMessageOnPort (node:internal/worker/io:410:19)
    at structuredClone (node:internal/structured_clone:24:10)
    at main (/home/h4ad/Projects/opensource/node-copy-3/benchmark/blob/clone.js:17:20)
    at /home/h4ad/Projects/opensource/node-copy-3/benchmark/common.js:54:9
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11)

Node.js v21.0.0-p

@rluvaton Do you have any hint?

[rluvaton]

On top of my head no but I will try to investigate it

I did not migrate transfer classes in webstream so did not encounter with this error

[H4ad]

The error of unable to deserialize cloned data could be a bug, I opened an issue nodejs/node#49844.

I discovered when I was trying to optimize the histogram, without changing the code, I got the error.

[H4ad]

Legendecas opened a PR to fix the issue described in the previous issue, nodejs/node#50026, I would appreciate it if anyone here could approve that PR, I'm already using it to remove usages of ReflectConstruct on transferable objects.

[H4ad]

All PRs were merged, I think I already removed all the relevant references for ReflectConstruct, so I think we can close this as completed.