GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — perf_hooks: add property flags to GCPerformanceEntry by fanatid · Pull Request #29547 · nodejs/node

Conversation

@fanatid

@fanatid fanatid commented Sep 13, 2019

Copy link
Copy Markdown
Contributor
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

perf_hooks have kind for GC, but do not have flags

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss and addaleax reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 13, 2019
fanatid
fanatid commented Sep 13, 2019
View reviewed changes
Comment thread doc/api/perf_hooks.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell approved these changes Sep 16, 2019
View reviewed changes
@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 16, 2019
@Trott
Trott force-pushed the master branch from 1ecc406 to 49cf67e Compare September 17, 2019 16:50
@addaleax addaleax added author ready PRs with CI started, the required approvals, and no outstanding review comments. perf_hooks Issues and PRs related to the perf_hooks module and performance measurement APIs. labels Nov 30, 2019
@nodejs-github-bot

nodejs-github-bot commented Nov 30, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27228/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Trott
Trott reviewed Dec 6, 2019
View reviewed changes
Comment thread doc/api/perf_hooks.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 6, 2019
@nodejs-github-bot

nodejs-github-bot commented Dec 6, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27410/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Dec 6, 2019

Copy link
Copy Markdown
Member

This could use more reviews. /cc @nodejs/performance
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax

addaleax commented Dec 6, 2019

Copy link
Copy Markdown
Member

This LGTM code-wise, but it doesn’t really seem to document what these flags indicate (although I guess the same is true for .kind…)
fanatid reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
joyeecheung
joyeecheung reviewed Dec 8, 2019
View reviewed changes
Comment thread doc/api/perf_hooks.md Outdated

@joyeecheung joyeecheung Dec 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If we are exposing this we at least need a note similar to the one in https://nodejs.org/api/v8.html#v8_v8_getheapspacestatistics

Neither the ordering of heap spaces, nor the availability of a heap space can be guaranteed as the statistics are provided via the V8 GetHeapSpaceStatistics function and may change from one V8 version to the next.

As compared to the GC kind the flags here seem to be even less stable.

This also lacks explanation and they are less self-explanatory compared to the gc kinds. I guess we could link to the v8 docs of GCCallbackFlags at least for reference.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
benjamingr reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@fanatid fanatid Dec 8, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This flags provided via V8 GCCallbackFlags enum and may change from one V8 version to next.

Looks good for adding after constants list?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@BridgeAR

BridgeAR commented Jan 12, 2020

Copy link
Copy Markdown
Member

This needs a rebase.
fanatid reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fanatid added 4 commits January 12, 2020 08:51
@fanatid
perf_hooks: add property flags to GCPerformanceEntry
ca67e58
@fanatid
perf_hooks: REPLACEME instead version in docs
f23b4ff
@fanatid
perf_hooks: fix lint-cpp
4b55aac
@fanatid
perf_hooks: fix REPLACEME in doc
36b01b7
@fanatid
fanatid force-pushed the perf_hooks-add-flags-for-gc branch from 86b0aa0 to 36b01b7 Compare January 12, 2020 05:52
@fanatid

fanatid commented Jan 12, 2020

Copy link
Copy Markdown
Contributor Author

Rebased.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Jan 12, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28398/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
BridgeAR
BridgeAR reviewed Jan 12, 2020
View reviewed changes
Comment thread test/parallel/test-performance-gc.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@fanatid
perf_hooks: remove empty line in test
d5e1a8a
@BridgeAR BridgeAR added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Jan 12, 2020
@nodejs-github-bot

nodejs-github-bot commented Jan 12, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28409/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
legendecas
legendecas approved these changes Feb 2, 2020
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Feb 2, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28872/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Feb 2, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28876/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Trott
Trott reviewed Feb 2, 2020
View reviewed changes
Comment thread doc/api/perf_hooks.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@fanatid
perf_hooks: adjust words in doc
bce3cb6
@nodejs-github-bot

nodejs-github-bot commented Feb 3, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28890/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Feb 3, 2020

Copy link
Copy Markdown
Member

Failure in CI does not appear relevant but kicked off a new CI just in case
fanatid reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Feb 3, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28899/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Feb 3, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28902/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax pushed a commit that referenced this pull request Feb 4, 2020
@fanatid @addaleax
perf_hooks: add property flags to GCPerformanceEntry
018c3e8 
PR-URL: #29547
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@addaleax

addaleax commented Feb 4, 2020

Copy link
Copy Markdown
Member

Landed in 018c3e8
fanatid reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax addaleax closed this Feb 4, 2020
@fanatid
fanatid deleted the perf_hooks-add-flags-for-gc branch February 4, 2020 16:43
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@fanatid @codebytere
perf_hooks: add property flags to GCPerformanceEntry
e7fea14 
PR-URL: #29547
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@codebytere codebytere mentioned this pull request Feb 17, 2020
v13.9.0 proposal #31837
Merged
codebytere added a commit that referenced this pull request Feb 18, 2020
@codebytere
2020-02-18, Version 13.9.0 (Current)
1f0f58c 
Notable changes:

* async_hooks
  * add executionAsyncResource (Matteo Collina) #30959
* crypto
  * add crypto.diffieHellman (Tobias Nießen) #31178
  * add DH support to generateKeyPair (Tobias Nießen) #31178
  * simplify DH groups (Tobias Nießen) #31178
  * add key type 'dh' (Tobias Nießen) #31178
* test
  * skip keygen tests on arm systems (Tobias Nießen) #31178
* perf_hooks
  * add property flags to GCPerformanceEntry (Kirill Fomichev) #29547
* process
  * report ArrayBuffer memory in `memoryUsage()` (Anna Henningsen) #31550
* readline
  * make tab size configurable (Ruben Bridgewater) #31318
* report
  * add support for Workers (Anna Henningsen) #31386
* worker
  * add ability to take heap snapshot from parent thread (Anna Henningsen) #31569
* added new collaborators
  * add ronag to collaborators (Robert Nagy) #31498

PR-URL: #31837
codebytere added a commit that referenced this pull request Feb 18, 2020
@codebytere
2020-02-18, Version 13.9.0 (Current)
0c3c0e7 
Notable changes:

* async_hooks
  * add executionAsyncResource (Matteo Collina) #30959
* crypto
  * add crypto.diffieHellman (Tobias Nießen) #31178
  * add DH support to generateKeyPair (Tobias Nießen) #31178
  * simplify DH groups (Tobias Nießen) #31178
  * add key type 'dh' (Tobias Nießen) #31178
* test
  * skip keygen tests on arm systems (Tobias Nießen) #31178
* perf_hooks
  * add property flags to GCPerformanceEntry (Kirill Fomichev) #29547
* process
  * report ArrayBuffer memory in `memoryUsage()` (Anna Henningsen) #31550
* readline
  * make tab size configurable (Ruben Bridgewater) #31318
* report
  * add support for Workers (Anna Henningsen) #31386
* worker
  * add ability to take heap snapshot from parent thread (Anna Henningsen) #31569
* added new collaborators
  * add ronag to collaborators (Robert Nagy) #31498

PR-URL: #31837
@targos targos removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@fanatid @targos
perf_hooks: add property flags to GCPerformanceEntry
055d054 
PR-URL: nodejs#29547
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
@fanatid @targos
perf_hooks: add property flags to GCPerformanceEntry
9eeee0d 
PR-URL: #29547
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@targos targos mentioned this pull request May 2, 2020
v12.17.0 release proposal #33197
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott left review comments
@joyeecheung joyeecheung joyeecheung left review comments
@richardlau richardlau richardlau left review comments
@BridgeAR BridgeAR BridgeAR left review comments
@jasnell jasnell jasnell approved these changes
@legendecas legendecas legendecas approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. perf_hooks Issues and PRs related to the perf_hooks module and performance measurement APIs. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@fanatid @nodejs-github-bot @Trott @addaleax @BridgeAR @jasnell @joyeecheung @richardlau @legendecas @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.