Skip to content

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Dec 22, 2017

Refactor and simplify the perf_hooks native internals.

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

perf_hooks

Refactor and simplify the perf_hooks native internals.
@jasnell jasnell added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Dec 22, 2017
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 22, 2017
@jasnell
Copy link
Member Author

jasnell commented Dec 22, 2017

src/node_perf.cc Outdated
v8::GCType performance_last_gc_type_ = v8::GCType::kGCTypeAll;

// Initialize the performance entry object properties
inline void InitObject(PerformanceEntry* entry, Local<Object> obj) {
Copy link
Member

Choose a reason for hiding this comment

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

const PerformanceEntry& entry? Makes it a bit more obvious which parameter is the source/target

src/node_perf.h Outdated
virtual ~PerformanceEntry() { }

~PerformanceEntry() {}
virtual Local<Object> ToObject();
Copy link
Member

Choose a reason for hiding this comment

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

This could/should be const as well I think?

@addaleax
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-commit/15081/

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 27, 2017
addaleax pushed a commit that referenced this pull request Dec 27, 2017
Refactor and simplify the perf_hooks native internals.

PR-URL: #17822
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax
Copy link
Member

Landed in 9e5ccf0

@addaleax addaleax closed this Dec 27, 2017
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 27, 2017
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
Refactor and simplify the perf_hooks native internals.

PR-URL: #17822
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Refactor and simplify the perf_hooks native internals.

PR-URL: #17822
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Refactor and simplify the perf_hooks native internals.

PR-URL: #17822
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
@gibfahn
Copy link
Member

gibfahn commented Jan 24, 2018

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

kjin pushed a commit to kjin/node that referenced this pull request May 1, 2018
Refactor and simplify the perf_hooks native internals.

PR-URL: nodejs#17822
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request May 2, 2018
Refactor and simplify the perf_hooks native internals.

Backport-PR-URL: #20456
PR-URL: #17822
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants