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

util: never trigger proxy traps while inspecting objects #30767

Closed
wants to merge 4 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Dec 2, 2019

Please have a look at the individual commits for details.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

This adds a couple of benchmarks to check different options and code
paths.
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.
@BridgeAR BridgeAR added util Issues and PRs related to the built-in util module. performance Issues and PRs related to the performance of Node.js. labels Dec 2, 2019
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 2, 2019
@nodejs-github-bot

This comment has been minimized.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 2, 2019
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 6, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/27425/ ✅ (yellow build with a known Windows flake)

BridgeAR added a commit that referenced this pull request Dec 6, 2019
This adds a couple of benchmarks to check different options and code
paths.

PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit that referenced this pull request Dec 6, 2019
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit that referenced this pull request Dec 6, 2019
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR
Copy link
Member Author

BridgeAR commented Dec 6, 2019

Landed in 832290a...e11b909 🎉

@BridgeAR BridgeAR closed this Dec 6, 2019
targos pushed a commit that referenced this pull request Dec 9, 2019
This adds a couple of benchmarks to check different options and code
paths.

PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Dec 9, 2019
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Dec 9, 2019
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos
Copy link
Member

targos commented Jan 14, 2020

Depends on #30343 to land on v12.x-staging

@BridgeAR BridgeAR deleted the inspect-prototype branch January 20, 2020 12:06
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 20, 2020
This adds a couple of benchmarks to check different options and code
paths.

PR-URL: nodejs#30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 20, 2020
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

PR-URL: nodejs#30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 20, 2020
PR-URL: nodejs#30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 22, 2020
This makes sure this function stays backwards compatible in case
it's accessed through the binding directly.

Refs: nodejs#29947 (comment)

PR-URL: nodejs#30858
Refs: nodejs#30767
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This adds a couple of benchmarks to check different options and code
paths.

Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This makes sure this function stays backwards compatible in case
it's accessed through the binding directly.

Refs: #29947 (comment)

Backport-PR-URL: #31431
PR-URL: #30858
Refs: #30767
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This adds a couple of benchmarks to check different options and code
paths.

Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
Backport-PR-URL: #31431
PR-URL: #30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This makes sure this function stays backwards compatible in case
it's accessed through the binding directly.

Refs: #29947 (comment)

Backport-PR-URL: #31431
PR-URL: #30858
Refs: #30767
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. performance Issues and PRs related to the performance of Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants