Going in the right direction! Feedback is mostly nits...


https://codereview.chromium.org/70013002/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/70013002/diff/40001/src/flag-definitions.h#newcode790
src/flag-definitions.h:790: DEFINE_bool(perf_basic_prof, false, "Enable
perf linux profiler (basic support).")
nit: 80 col (tools/presubmit should have pointed this out, BTW)

https://codereview.chromium.org/70013002/diff/40001/src/flag-definitions.h#newcode791
src/flag-definitions.h:791: DEFINE_bool(perf_jit_prof, false, "Enable
perf linux profiler "
How about call this one perf_annotate_support?

https://codereview.chromium.org/70013002/diff/40001/src/flag-definitions.h#newcode792
src/flag-definitions.h:792: "(experimental annotate support).")
You probably want to make sure that these flags imply
nocompact_code_space, or at least in
MarkCompactCollector::StartCompaction make sure, just like with the
gdbjit, that when these features are on, there is no compaction
possible.

https://codereview.chromium.org/70013002/diff/40001/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/70013002/diff/40001/src/log.cc#newcode252
src/log.cc:252: explicit PerfBasicLogger();
Don't need explicit here, only for single argument constructors.

https://codereview.chromium.org/70013002/diff/40001/src/log.cc#newcode285
src/log.cc:285: perf_dump_name,
nit: 4 char indent

https://codereview.chromium.org/70013002/diff/40001/src/log.cc#newcode307
src/log.cc:307: (uint64_t)code->instruction_start(),
nit: 4 char indent (2 is for nested scopes only)

https://codereview.chromium.org/70013002/diff/40001/src/log.cc#newcode316
src/log.cc:316: explicit PerfJitLogger();
Don't need explicit here, only for single argument constructor.

https://codereview.chromium.org/70013002/diff/40001/src/log.cc#newcode404
src/log.cc:404: perf_dump_name,
Indent 4 characters on line continuations

https://codereview.chromium.org/70013002/diff/40001/src/log.cc#newcode422
src/log.cc:422: SharedFunctionInfo*,
nit: indentation one char too many on this line and below.

https://codereview.chromium.org/70013002/

--
--
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/groups/opt_out.

Reply via email to