Thanks, Søren and William!

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")
On 2009/05/25 07:09:15, Søren Gjesse wrote:
> 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.

Currently, we have several differences in results of "traditional"
compiler / gc logging vs. heap traversal, the differences can be seen in
the code of "AreFuncNamesEqual" and "AreEntitiesEqual" functions from
test-log.cc. They only related to native and internal code, so they are
irrelevant to web app developers, but may make sense for V8 developers.
That's why I'll prefer to leave "traditional" profiling logging for some
time until I figure out how to handle this better.

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();
On 2009/05/25 07:09:15, Søren Gjesse wrote:
> 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.

That's an important thing to know, thanks for noticing. I removed flag
usage from here, and also added checking of FLAG_sliding_state_window in
Pause/ResumeProfiler functions to avoid stopping/restarting Ticker when
SSW is enabled.

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>
On 2009/05/25 07:09:15, Søren Gjesse wrote:
> Why is this a template function? T needs to be char for
Logger::GetLogLines to
> match.

Yep, you're right. Fixed.

http://codereview.chromium.org/113762

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

Reply via email to