Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine globals and specialFunctions #563

Open
overlookmotel opened this issue Dec 18, 2023 · 0 comments
Open

Combine globals and specialFunctions #563

overlookmotel opened this issue Dec 18, 2023 · 0 comments
Labels
enhancement Improvements

Comments

@overlookmotel
Copy link
Owner

overlookmotel commented Dec 18, 2023

When serializing any non-primitive value, it is looked up in the globals Map, to check if it's a global property or property of a NodeJS built-in module e.g. Object.is or require('path').join.

Functions are then checked against the specialFunctions Map to check if it's a bound function, or other special functions e.g. a require function.

The original rationale for not adding these "special functions" into globals is that specialFunctions is a WeakMap, and so does not prevent them being garbage collected.

However, it's wasteful to have 2 Map lookups for every function which is serialized.

Avoid this by either:

  1. Making globals a WeakMap. Then add "special functions" into globals.
  2. Add globals which are functions into specialFunctions instead of globals. So then a function being serialized only needs to be looked up once in the specialFunctions Map.

I don't think making globals a WeakMap would change much. All globals are strongly held anyway, and I think NodeJS's built-in modules are too.

Only exceptions I can see are:

  1. undefined which is currently stored as a global, but can't be a WeakMap key.
  2. Global Symbols, which I can't be WeakMap keys in NodeJS v18.
  3. Global well-known Symbols (Symbol.for('...')) which can't be WeakMap keys in any version of NodeJS. But these shouldn't be added to globals anyway, so they're serialized as e.g. Symbol.for('nodejs.util.promisify.custom') instead of require('util').promisify.custom.
  4. The 3 example async/generator functions:

for (const [fn, name] of [
[function*() {}, 'generatorFunction'], // eslint-disable-line no-empty-function
[async function() {}, 'asyncFunction'], // eslint-disable-line no-empty-function
[async function*() {}, 'asyncGeneratorFunction'] // eslint-disable-line no-empty-function
]) {
// Replace existing references to these prototypes which were found in odd places
// e.g. `Object.getPrototypeOf(require("fs").Dir.prototype.entries)`
globals.delete(getPrototypeOf(fn));
if (fn.prototype) globals.delete(getPrototypeOf(fn.prototype));
addToQueue(fn, SPECIAL, null, name, false, queue);
}

@overlookmotel overlookmotel added the enhancement Improvements label Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements
Projects
None yet
Development

No branches or pull requests

1 participant