Thank you for your prompt review.
I have addressed the issues you mentioned.
Up to patch 5, I have addressed the issues without moving the V8
implementation
to a callback.
Patch 6 moves the V8 implementation to a default callback.
PTAL.
https://codereview.chromium.org/186163002/diff/60001/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/186163002/diff/60001/src/api.cc#newcode406
src/api.cc:406: isolate->set_event_logger(that);
On 2014/03/05 07:47:29, Sven Panne wrote:
This line is a *very* strong hint that SetEventLogger is in the wrong
class: It
should be a normal member function within Isolate. Furthermore, using
EnterIsolateIfNeeded is a hack and should not be used.
Done.
https://codereview.chromium.org/186163002/diff/60001/src/log.cc
File src/log.cc (right):
https://codereview.chromium.org/186163002/diff/60001/src/log.cc#newcode1128
src/log.cc:1128: if (FLAG_log_internal_timer_events)
On 2014/03/05 08:18:31, Yang wrote:
We always use brackets if an if-clause contains a linebreak. Either
use brackets
or don't linebreak please.
Fixed in Patch 5, removed in Patch 6.
https://codereview.chromium.org/186163002/diff/60001/src/log.h
File src/log.h (right):
https://codereview.chromium.org/186163002/diff/60001/src/log.h#newcode104
src/log.h:104: (isolate)->event_logger()(name, 0)
On 2014/03/05 08:18:31, Yang wrote:
On 2014/03/05 07:47:29, Sven Panne wrote:
> Parenthesize "name" and never ever use unbalanced "if"s in macros.
> Alternatively: Why don't we just use an inline function instead of
the macro?
> This would be nicer...
Agree.
Moved to an inlined function in Patch 5, and totally removed in Patch 6.
https://codereview.chromium.org/186163002/diff/60001/src/log.h#newcode330
src/log.h:330: LogTimerEvent(START);
On 2014/03/05 08:18:31, Yang wrote:
I'm concerned that this may regress performance since we call
LogTimerEvent
(which is not inlined) even if the flag is off.
As you mentioned, I am only including those for the
Flag_log_internal_timer_events, which should not have a measurable
overhead (I am also excluding it from External).
https://codereview.chromium.org/186163002/
--
--
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.
