Logger: introduce abstract interface for CodeEvent listeners.

Logger: introduce abstract interface for CodeEvent listeners.

New abstract class CodeEventListener was created.

CodeEventLogger which is the base class for Jit, LowLevel
and CodeAddressMap loggers was inherited from CodeEventListener.

CodeAddressMap class was moved to serializer.cc because serializer is the only user for it. Actually it collects code names and pushes them to the standard log as SnapshotCodeNameEvent. So I extracted this code into separate function CodeNameEvent. It happens that this method works only when Serializer serializes an object. So I added direct log call there.

CodeEventLogger class declaration was moved to the header
because CodeAddressMap needs it.
The code for the nested class CodeEventLogger::NameBuffer was left in the cc file.

CpuProfiler now is inherit CodeEventListener but not used
the loggers infrastructure yet due to the complex initialization schema. I'd like to fix that in a separate cl.

BUG=none
TEST=current test set.
R=yangguo@chromium.org, yurys@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15911

[yurys, 2013-07-24 09:15:40]

https://codereview.chromium.org/19724007/diff/1/src/cpu-profiler.h
File src/cpu-profiler.h (right):

https://codereview.chromium.org/19724007/diff/1/src/cpu-profiler.h#newcode187
src/cpu-profiler.h:187: class CpuProfiler : public CodeEventLoggerBase {
Should we add virtual to the overrides for clarity?

https://codereview.chromium.org/19724007/diff/1/src/log.h
File src/log.h (right):

https://codereview.chromium.org/19724007/diff/1/src/log.h#newcode505
src/log.h:505: class CodeEventLogger : public CodeEventLoggerBase {
CodeEventLoggerBase should be CodeEventLogger, and this one should be
CodeEventLoggerImplBase or something like that.

[loislo, 2013-07-24 09:50:34]

On 2013/07/24 09:15:40, Yury Semikhatsky wrote:
> https://codereview.chromium.org/19724007/diff/1/src/cpu-profiler.h
> File src/cpu-profiler.h (right):
> 
> https://codereview.chromium.org/19724007/diff/1/src/cpu-profiler.h#newcode187
> src/cpu-profiler.h:187: class CpuProfiler : public CodeEventLoggerBase {
> Should we add virtual to the overrides for clarity?
> 
> https://codereview.chromium.org/19724007/diff/1/src/log.h
> File src/log.h (right):
> 
> https://codereview.chromium.org/19724007/diff/1/src/log.h#newcode505
> src/log.h:505: class CodeEventLogger : public CodeEventLoggerBase {
> CodeEventLoggerBase should be CodeEventLogger, and this one should be
> CodeEventLoggerImplBase or something like that.

comments addressed

[yurys, 2013-07-24 12:02:34]

https://codereview.chromium.org/19724007/diff/5001/src/log.h
File src/log.h (right):

https://codereview.chromium.org/19724007/diff/5001/src/log.h#newcode228
src/log.h:228: void addLogger(CodeEventLogger* logger);
I'd rename these to add/removeCodeEventListener and the interface to
CodeEventListener, wdyt?

[loislo, 2013-07-24 12:15:14]

On 2013/07/24 12:02:34, Yury Semikhatsky wrote:
> https://codereview.chromium.org/19724007/diff/5001/src/log.h
> File src/log.h (right):
> 
> https://codereview.chromium.org/19724007/diff/5001/src/log.h#newcode228
> src/log.h:228: void addLogger(CodeEventLogger* logger);
> I'd rename these to add/removeCodeEventListener and the interface to
> CodeEventListener, wdyt?

done

[yurys, 2013-07-26 08:04:54]

https://codereview.chromium.org/19724007/diff/10001/src/log.h
File src/log.h (right):

https://codereview.chromium.org/19724007/diff/10001/src/log.h#newcode228
src/log.h:228: void addLogger(CodeEventListener* listener);
addCodeEventListener

https://codereview.chromium.org/19724007/diff/10001/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/19724007/diff/10001/src/serialize.cc#newcode786
src/serialize.cc:786: }
Should return early in else branch.

https://codereview.chromium.org/19724007/diff/10001/src/serialize.cc#newcode795
src/serialize.cc:795: serialization_enabled_ = false;
ASSERT(serialization_enabled_) ?

[loislo, 2013-07-26 08:18:59]

On 2013/07/26 08:04:54, Yury Semikhatsky wrote:
> https://codereview.chromium.org/19724007/diff/10001/src/log.h
> File src/log.h (right):
> 
> https://codereview.chromium.org/19724007/diff/10001/src/log.h#newcode228
> src/log.h:228: void addLogger(CodeEventListener* listener);
> addCodeEventListener

done

> 
> https://codereview.chromium.org/19724007/diff/10001/src/serialize.cc
> File src/serialize.cc (right):
> 
>
https://codereview.chromium.org/19724007/diff/10001/src/serialize.cc#newcode786
> src/serialize.cc:786: }
> Should return early in else branch.

done

> 
>
https://codereview.chromium.org/19724007/diff/10001/src/serialize.cc#newcode795
> src/serialize.cc:795: serialization_enabled_ = false;
> ASSERT(serialization_enabled_) ?

I inserted early return.

[yurys, 2013-07-26 08:39:55]

https://codereview.chromium.org/19724007/diff/16001/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/19724007/diff/16001/src/serialize.cc#newcode1598
src/serialize.cc:1598: LOG(i::Isolate::Current(),
Please use isolate_ instead of i::Isolate::Current()

[yurys, 2013-07-26 08:41:08]

lgtm

[Yang, 2013-07-26 12:49:59]

On 2013/07/26 08:41:08, Yury Semikhatsky wrote:
> lgtm

I agree with Yury's last comment. LGTM otherwise.

[loislo, 2013-07-26 13:19:55]

On 2013/07/26 12:49:59, Yang wrote:
> On 2013/07/26 08:41:08, Yury Semikhatsky wrote:
> > lgtm
> 
> I agree with Yury's last comment. LGTM otherwise.

It is static function so, it has no access to the non-static member isolate_.

[Yang, 2013-07-26 13:36:46]

On 2013/07/26 13:19:55, loislo wrote:
> On 2013/07/26 12:49:59, Yang wrote:
> > On 2013/07/26 08:41:08, Yury Semikhatsky wrote:
> > > lgtm
> > 
> > I agree with Yury's last comment. LGTM otherwise.
> 
> It is static function so, it has no access to the non-static member isolate_.

I don't think it's static. You could get the isolate with serializer_->isolate_.

[loislo, 2013-07-26 13:50:31]

Committed patchset #5 manually as r15911 (presubmit successful).

[Yang, 2013-07-26 13:50:34]

On 2013/07/26 13:36:46, Yang wrote:
> On 2013/07/26 13:19:55, loislo wrote:
> > On 2013/07/26 12:49:59, Yang wrote:
> > > On 2013/07/26 08:41:08, Yury Semikhatsky wrote:
> > > > lgtm
> > > 
> > > I agree with Yury's last comment. LGTM otherwise.
> > 
> > It is static function so, it has no access to the non-static member
isolate_.
> 
> I don't think it's static. You could get the isolate with
serializer_->isolate_.

lgtm.