lib: use more primordials by aduh95 · Pull Request #35838 · nodejs/node · GitHub

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

Review requested:

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

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

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

node/lib/internal/per_context/primordials.js

Lines 22 to 24 in 802c98d

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

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

I just realized there are some unnecessary whitespace changes.

Needs a rebase.

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

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

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

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2020

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Oct 27, 2020

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2020

ExE-Boss reviewed Oct 29, 2020

View reviewed changes

Comment thread lib/_http_client.js Outdated

aduh95 force-pushed the refactor-function-calls-primordials branch from 3b56f87 to 57c325a Compare October 29, 2020 20:13

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2020

aduh95 marked this pull request as ready for review October 29, 2020 22:02

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2020

This comment has been minimized.

Sign in to view

aduh95 commented Oct 29, 2020

View reviewed changes

Comment thread lib/dgram.js Outdated

Comment thread lib/internal/http2/core.js Outdated

Comment thread lib/internal/http2/core.js Outdated

Comment thread lib/internal/http2/core.js Outdated

Comment thread lib/internal/http2/core.js Outdated

Comment thread lib/internal/quic/core.js Outdated

aduh95 mentioned this pull request Oct 30, 2020

domain: refactor to use more primordials #35885

Closed

2 tasks

aduh95 force-pushed the refactor-function-calls-primordials branch from 7ec460c to 8c2c47a Compare October 30, 2020 15:17

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2020

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2020

This comment has been minimized.

Sign in to view

aduh95 added the review wanted PRs that need reviews. label Nov 1, 2020

aduh95 force-pushed the refactor-function-calls-primordials branch from 8c2c47a to a3d70cd Compare November 3, 2020 09:21

lib: use more primordials

882e66e

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

aduh95 force-pushed the refactor-function-calls-primordials branch from a3d70cd to 882e66e Compare November 6, 2020 10:29

aduh95 closed this Nov 7, 2020

aduh95 deleted the refactor-function-calls-primordials branch November 7, 2020 14:19

aduh95 mentioned this pull request Nov 8, 2020

net: Fix invalid write after end error #36043

Closed

4 tasks

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

lib: use more primordials by aduh95 · Pull Request #35838 · nodejs/node · GitHub

Uh oh!

Uh oh!

lib: use more primordials - #35838

lib: use more primordials#35838aduh95 wants to merge 1 commit intonodejs:masternodejs/node:masterfrom aduh95:refactor-function-calls-primordialsaduh95/node:refactor-function-calls-primordialsCopy head branch name to clipboard

Conversation

aduh95 commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

nodejs-github-bot commented Oct 27, 2020

Uh oh!

nodejs-github-bot commented Oct 27, 2020

Uh oh!

ExE-Boss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

nodejs-github-bot commented Oct 29, 2020

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trott commented Oct 30, 2020

Uh oh!

This comment has been minimized.

nodejs-github-bot commented Oct 31, 2020 • edited by aduh95 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aduh95 commented Nov 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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

aduh95 commented Oct 27, 2020 •
edited
Loading

nodejs-github-bot commented Oct 31, 2020 •
edited by aduh95
Loading