LGTM


http://codereview.chromium.org/113762/diff/25/1005
File src/flag-definitions.h (right):

http://codereview.chromium.org/113762/diff/25/1005#newcode338
Line 338: "Used with --prof, starts profiling automatically")
Do we need both --prof-auto and --prof-lazy. With your last change where
logging can be initiated by getting all the functions in the heap I
don't see a reason for logging and sampling if not really profiling. Am
I missing something here? If it makes sense to consolidate --prof-auto
and --prof-lazy in some way you can do that in a new change.

http://codereview.chromium.org/113762/diff/25/1006
File src/log.cc (right):

http://codereview.chromium.org/113762/diff/25/1006#newcode181
Line 181: if (!FLAG_prof_lazy && !IsActive()) Start();
I am not sure about this handling of the prof_lazy flag. The main reason
for implementing the sliding window average was to enable some display
of it in Chrome. This should be unrelated to dev tools - maybe in the
task manager. This is still not implemented, so I guess this is fine for
now.

http://codereview.chromium.org/113762/diff/25/1009
File test/cctest/test-log.cc (right):

http://codereview.chromium.org/113762/diff/25/1009#newcode88
Line 88: template <typename T>
Why is this a template function? T needs to be char for
Logger::GetLogLines to match.

http://codereview.chromium.org/113762

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to