Idea: if we assume that we are never going to log both to V8 and through the
callback at the same time, we could implement the existing logging as a
callback
that is installed by default. In that case we don't have to check whether
the
callback exists, and just call it. The API would replace that default
callback.
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.
I agree. I would also suggest adding it to V8::Isolate.
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)
We always use brackets if an if-clause contains a linebreak. Either use
brackets or don't linebreak please.
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 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.
https://codereview.chromium.org/186163002/diff/60001/src/log.h#newcode330
src/log.h:330: LogTimerEvent(START);
I'm concerned that this may regress performance since we call
LogTimerEvent (which is not inlined) even if the flag is off.
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.
