Skip to content

net: improve performance by destructuring primordials - #30447

Closed
ghost wants to merge 4 commits into
masterfrom
unknown repository
Closed

net: improve performance by destructuring primordials#30447
ghost wants to merge 4 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghostghost commented Nov 12, 2019

Copy link
Copy Markdown

Refs: #29766

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

@nodejs-github-botnodejs-github-bot added the net Issues and PRs related to the net subsystem. label Nov 12, 2019
@gireeshpunathilgireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 12, 2019
@ghost

ghost commented Nov 12, 2019

Copy link
Copy Markdown
Author

I'm actually getting a linter error on a line I didn't change (L538):

defineProperty(Socket.prototype,'bufferSize',{get: function(){// eslint-disable-line getter-returnif(this._handle){returnthis[kLastWriteQueueSize]+this.writableLength;}}});

Should I address that as part of this PR as well?

lundibundi
lundibundi reviewed Nov 14, 2019
View reviewed changes
Comment threadlib/net.js Outdated
Comment threadlib/net.js
@lundibundi

lundibundi commented Nov 15, 2019

Copy link
Copy Markdown
Member

Also, it looks like we usually go with capitalized Object... in such cases, could you please replace them objectSetPrototypeOf -> ObjectSetPrototypeOf and such? Sorry for the typo in the first comment.

@nodejs-github-bot

nodejs-github-bot commented Nov 17, 2019

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

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

@ghost

ghost commented Nov 18, 2019

Copy link
Copy Markdown
Author

Also, it looks like we usually go with capitalized Object... in such cases, could you please replace them objectSetPrototypeOf -> ObjectSetPrototypeOf and such?

Ooops, renamed. Thanks for bearing with me!

@nodejs-github-bot

nodejs-github-bot commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

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

gireeshpunathil pushed a commit that referenced this pull request Nov 19, 2019
Guilherme Goncalves@gireeshpunathil
net: destructure primordials
5e4d99a
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@gireeshpunathil

gireeshpunathil commented Nov 19, 2019

Copy link
Copy Markdown
Member

landed in 5e4d99a, thanks for the contribution!

BridgeAR pushed a commit that referenced this pull request Nov 19, 2019
Guilherme Goncalves@BridgeAR
net: destructure primordials
66e1adf
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BridgeARBridgeAR mentioned this pull request Nov 19, 2019
Merged
targos pushed a commit that referenced this pull request Dec 1, 2019
Guilherme Goncalves@targos
net: destructure primordials
6894703
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Dec 9, 2019
Closed
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
Guilherme Goncalves@MylesBorins
net: destructure primordials
0ae1d17
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Dec 23, 2019
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

code-and-learnIssues related to the Code-and-Learn events and PRs submitted during the events.netIssues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@lundibundi@nodejs-github-bot@gireeshpunathil@addaleax@cjihrig@trivikr