Skip to content

Conversation

matthewloring
Copy link

src/tracing/trace_event.h was updated by applying the following changes:

  1. Include src/tracing/trace_event_common.h instead of the one from v8
    base.

  2. Replace all instances of base::Atomic with intptr_t (trace events can
    only be generated from the main thread for now).

  3. Replace instances of V8_INLINE with inline.

  4. Eliminate uses of DCHECK.

  5. Eliminate uses of V8_UNLIKELY, the branch predictor should be good
    enough alone.

  6. Change the namespace used by trace_event.h from v8::internal::tracing
    to node::tracing.

  7. Remove CallStatsScopedTracer class and related macros (they rely on
    V8 implementation details).

  8. Change ConvertableToTraceFormat to v8::ConvertableToTraceFormat.

  9. Add function "static void SetCurrentPlatform(v8::Platform*
    platform);" to the declaration of TraceEventHelper.

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

src, tracing

src/tracing/trace_event.h was updated by applying the following changes:

1) Include src/tracing/trace_event_common.h instead of the one from v8
base.

2) Replace all instances of base::Atomic with intptr_t (trace events can
only be generated from the main thread for now).

3) Replace instances of V8_INLINE with inline.

4) Eliminate uses of DCHECK.

5) Eliminate uses of V8_UNLIKELY, the branch predictor should be good
enough alone.

6) Change the namespace used by trace_event.h from v8::internal::tracing
to node::tracing.

7) Remove CallStatsScopedTracer class and related macros (they rely on
V8 implementation details).

8) Change ConvertableToTraceFormat to v8::ConvertableToTraceFormat.

9) Add function "static void SetCurrentPlatform(v8::Platform*
platform);" to the declaration of TraceEventHelper.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. labels Mar 29, 2017
@matthewloring
Copy link
Author

/cc @targos @joshgav @ofrobots

@joshgav
Copy link
Contributor

joshgav commented Mar 29, 2017

haven't reviewed the code yet really, but can confirm it does track AsyncWrap objects with joshgav@fda406f. Thanks @matthewloring!

@matthewloring
Copy link
Author

@nodejs/v8 to review

Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

Rubberstamp LGTM

@matthewloring
Copy link
Author

@matthewloring
Copy link
Author

CI is green

@matthewloring
Copy link
Author

Landed in ebeee85

@jasnell jasnell mentioned this pull request May 11, 2017
hferreiro pushed a commit to brave/node that referenced this pull request Sep 27, 2017
src/tracing/trace_event.h was updated by applying the following changes:

1) Include src/tracing/trace_event_common.h instead of the one from v8
base.

2) Replace all instances of base::Atomic with intptr_t (trace events can
only be generated from the main thread for now).

3) Replace instances of V8_INLINE with inline.

4) Eliminate uses of DCHECK.

5) Eliminate uses of V8_UNLIKELY, the branch predictor should be good
enough alone.

6) Change the namespace used by trace_event.h from v8::internal::tracing
to node::tracing.

7) Remove CallStatsScopedTracer class and related macros (they rely on
V8 implementation details).

8) Change ConvertableToTraceFormat to v8::ConvertableToTraceFormat.

9) Add function "static void SetCurrentPlatform(v8::Platform*
platform);" to the declaration of TraceEventHelper.

PR-URL: nodejs/node#12127
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
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++. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants