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

lib: use more primordials #35838

Closed

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Oct 27, 2020

This replaces all instances of Function.prototype.apply, Function.prototype.bind, and Function.prototype.call to their primordials alter ego.

I've used search and replace to make this PR, I've excluded changes in files that are already covered by other PRs (#35885, #35875, #35734). No test is being affected by this PR.

ReflectApply is used to replace Function.prototype.apply because it has less overhead (see #35838 (review)). There is an argument to be made that all Function.prototype.call calls could also be replaced by ReflectApply, but that's not done in this PR.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2020
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Oct 27, 2020
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/net
  • @nodejs/startup

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2020
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of FunctionPrototypeApply, this should use ReflectApply, which doesn’t go through the uncurryThis closure:

function uncurryThis(func) {
return (thisArg, ...args) => ReflectApply(func, thisArg, args);
}

lib/_http_client.js Outdated Show resolved Hide resolved
@aduh95 aduh95 force-pushed the refactor-function-calls-primordials branch from 3b56f87 to 57c325a Compare October 29, 2020 20:13
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2020
@aduh95 aduh95 marked this pull request as ready for review October 29, 2020 22:02
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2020
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor Author

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized there are some unnecessary whitespace changes.

lib/dgram.js Outdated Show resolved Hide resolved
lib/internal/http2/core.js Outdated Show resolved Hide resolved
lib/internal/http2/core.js Outdated Show resolved Hide resolved
lib/internal/http2/core.js Outdated Show resolved Hide resolved
lib/internal/http2/core.js Outdated Show resolved Hide resolved
lib/internal/quic/core.js Outdated Show resolved Hide resolved
@Trott
Copy link
Member

Trott commented Oct 30, 2020

Needs a rebase.

@aduh95 aduh95 force-pushed the refactor-function-calls-primordials branch from 7ec460c to 8c2c47a Compare October 30, 2020 15:17
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2020
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 31, 2020

@aduh95 aduh95 added the review wanted PRs that need reviews. label Nov 1, 2020
@aduh95 aduh95 force-pushed the refactor-function-calls-primordials branch from 8c2c47a to a3d70cd Compare November 3, 2020 09:21
This replaces all Function.prototype.apply, Function.prototype.bind,
Function.prototype.call to their primordials alter ego.
@aduh95 aduh95 force-pushed the refactor-function-calls-primordials branch from a3d70cd to 882e66e Compare November 6, 2020 10:29
@aduh95
Copy link
Contributor Author

aduh95 commented Nov 7, 2020

I'm going to split it into smaller PRs, that's too many files to review for a single PR of that kind.

@aduh95 aduh95 closed this Nov 7, 2020
@aduh95 aduh95 deleted the refactor-function-calls-primordials branch November 7, 2020 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. review wanted PRs that need reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants