Addressed Dan's comments.
Yang, PTAL!
https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h
File include/v8-tracing.h (right):
https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode32
include/v8-tracing.h:32: // These values must be in sync with macro
values in trace_event.h in
On 2015/08/25 13:56:22, dsinclair wrote:
nit: s/trace_event.h/trace_log.h
Done.
https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode41
include/v8-tracing.h:41: };
On 2015/08/25 13:56:22, dsinclair wrote:
Should we add a placeholder for ETW_EXPORT 1 << 3 here? Or comment
that it's
used?
I don't think there is a problem of just adding it?
https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode59
include/v8-tracing.h:59: };
On 2015/08/25 13:56:22, dsinclair wrote:
DISALLOW_COPY_AND_ASSIGN?
Done.
https://codereview.chromium.org/988893003/diff/240001/include/v8-tracing.h#newcode60
include/v8-tracing.h:60: } // namespace v8
On 2015/08/25 13:56:22, dsinclair wrote:
nit: blank line above
Done.
https://codereview.chromium.org/988893003/diff/240001/src/tracing/event-tracer.cc
File src/tracing/event-tracer.cc (right):
https://codereview.chromium.org/988893003/diff/240001/src/tracing/event-tracer.cc#newcode44
src/tracing/event-tracer.cc:44: }
On 2015/08/25 13:56:22, dsinclair wrote:
nit: no {}'s on single line bodies
Done.
https://codereview.chromium.org/988893003/diff/240001/src/tracing/trace-event.h
File src/tracing/trace-event.h (right):
https://codereview.chromium.org/988893003/diff/240001/src/tracing/trace-event.h#newcode60
src/tracing/trace-event.h:60:
On 2015/08/25 13:56:22, dsinclair wrote:
Should this be UNIMPLEMENTED()?
No, because it is OK for it to have no implementation, as it gets called
from all trace events.
For those trace events, they will work as expected except without memory
tracing.
Added a comment.
https://codereview.chromium.org/988893003/diff/240001/src/tracing/trace-event.h#newcode111
src/tracing/trace-event.h:111: // FIXME
On 2015/08/25 13:56:22, dsinclair wrote:
crbug?
I fixed the implementation, was a left out of a copy from Skia that has
long been fix, thanks for the catch.
https://codereview.chromium.org/988893003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
