Skip to content

lib: use more primordials - #35838

Closed
aduh95 wants to merge 1 commit into
nodejs:masterfrom
aduh95:refactor-function-calls-primordials
Closed

lib: use more primordials#35838
aduh95 wants to merge 1 commit into
nodejs:masterfrom
aduh95:refactor-function-calls-primordials

Conversation

@aduh95

@aduh95aduh95 commented Oct 27, 2020

Copy link
Copy Markdown
Contributor

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

@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2020
@nodejs-github-botnodejs-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
Copy Markdown
Collaborator

Review requested:

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

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

Copy link
Copy Markdown
Collaborator

@ExE-BossExE-Boss left a comment

Copy link
Copy Markdown
Contributor

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:

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

Comment threadlib/_http_client.js Outdated
@aduh95
aduh95force-pushed the refactor-function-calls-primordials branch from 3b56f87 to 57c325aCompareOctober 29, 2020 20:13
@aduh95aduh95 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-actionsgithub-actionsBot 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
Copy Markdown
Collaborator

@aduh95aduh95 left a comment

Copy link
Copy Markdown
ContributorAuthor

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.

Comment threadlib/dgram.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/quic/core.js Outdated
@Trott

Copy link
Copy Markdown
Member

Needs a rebase.

@aduh95aduh95 mentioned this pull request Oct 30, 2020
2 tasks
@aduh95
aduh95force-pushed the refactor-function-calls-primordials branch from 7ec460c to 8c2c47aCompareOctober 30, 2020 15:17
@TrottTrott added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2020
@github-actionsgithub-actionsBot 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

nodejs-github-bot commented Oct 31, 2020

Copy link
Copy Markdown
Collaborator

@aduh95aduh95 added the review wanted PRs that need reviews. label Nov 1, 2020
@aduh95
aduh95force-pushed the refactor-function-calls-primordials branch from 8c2c47a to a3d70cdCompareNovember 3, 2020 09:21
This replaces all Function.prototype.apply, Function.prototype.bind,
Function.prototype.call to their primordials alter ego.
@aduh95
aduh95force-pushed the refactor-function-calls-primordials branch from a3d70cd to 882e66eCompareNovember 6, 2020 10:29
@aduh95

Copy link
Copy Markdown
ContributorAuthor

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

@aduh95aduh95 closed this Nov 7, 2020
@aduh95
aduh95 deleted the refactor-function-calls-primordials branch November 7, 2020 14:19
@aduh95aduh95 mentioned this pull request Nov 8, 2020
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / srcIssues and PRs related to general changes in the lib or src directory.review wantedPRs that need reviews.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@aduh95@nodejs-github-bot@Trott@ExE-Boss