★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/29547Nouvelle recherche | Portail wanayoo
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

perf_hooks: add property flags to GCPerformanceEntry #29547

Closed
wants to merge 6 commits into from

Conversation

Copy link
Contributor

@fanatid fanatid commented Sep 13, 2019

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

@nodejs-github-bot nodejs-github-bot added the c++ label Sep 13, 2019
doc/api/perf_hooks.md Outdated Show resolved Hide resolved
@jasnell jasnell added the semver-minor label Sep 16, 2019
@addaleax addaleax added author ready perf_hooks labels Nov 30, 2019
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Nov 30, 2019

doc/api/perf_hooks.md Outdated Show resolved Hide resolved
@Trott Trott removed the author ready label Dec 6, 2019
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Dec 6, 2019

@Trott
Copy link
Member

@Trott Trott commented Dec 6, 2019

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

@addaleax
Copy link
Member

@addaleax addaleax commented Dec 6, 2019

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…)


When `performanceEntry.entryType` is equal to `'gc'`, the `performance.flags`
property contain additional information about garbage collection operation.
The value may be one of:
Copy link
Member

@joyeecheung joyeecheung Dec 8, 2019

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.

Copy link
Contributor Author

@fanatid fanatid Dec 8, 2019

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?

@BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Jan 12, 2020

This needs a rebase.

@fanatid fanatid force-pushed the perf_hooks-add-flags-for-gc branch from 86b0aa0 to 36b01b7 Compare Jan 12, 2020
@fanatid
Copy link
Contributor Author

@fanatid fanatid commented Jan 12, 2020

Rebased.

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 12, 2020

@BridgeAR BridgeAR added the author ready label Jan 12, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 12, 2020

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Feb 2, 2020

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Feb 2, 2020

doc/api/perf_hooks.md Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Feb 3, 2020

@jasnell
Copy link
Member

@jasnell jasnell commented Feb 3, 2020

Failure in CI does not appear relevant but kicked off a new CI just in case

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Feb 3, 2020

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Feb 3, 2020

addaleax pushed a commit that referenced this issue Feb 4, 2020
PR-URL: #29547
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@addaleax
Copy link
Member

@addaleax addaleax commented Feb 4, 2020

Landed in 018c3e8

@addaleax addaleax closed this Feb 4, 2020
@fanatid fanatid deleted the perf_hooks-add-flags-for-gc branch Feb 4, 2020
codebytere pushed a commit that referenced this issue Feb 17, 2020
PR-URL: #29547
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@codebytere codebytere mentioned this pull request Feb 17, 2020
codebytere added a commit that referenced this issue Feb 18, 2020
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 issue Feb 18, 2020
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 label Apr 25, 2020
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
PR-URL: nodejs#29547
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit that referenced this issue Apr 28, 2020
PR-URL: #29547
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ perf_hooks semver-minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants