On 1/13/11 7:39 AM, "[email protected]" <[email protected]> wrote:
> Hi Mark,
> 
> I looked at the code, and I think I built up an understanding of
> your approach, and it looks great.  For the purpose of
> committing, please separate your CL into smaller pieces and try
> to write some unit tests. I can propose the following partition:
>    - list.h, list-inl.h (bsearch)
>    - utils.cc, v8utils.h, platform.h, .cc (mmapped ext. strings -- nice idea,
> btw!)
>    - liveobjectlist.h, .cc, flag-definitions.h
>    - inspector.h, .cc
>    - heap.cc, mark-compact.cc, spaces.cc (wirings)
>    - runtime.h, .cc, debug-debugger.js, d8.cc, .js (interface)
>    - SConstruct, SConscript (turn on)

Thanks.  I'll prepare that.

> Some general comments:
> 
> 1. Please run tools/presubmit.py to fix all easy to find style violations.

Thanks, I'll try that.  Did you find anymore style violations?  I tried to
fix all the ones that the review tool found, but I see that it complains
about the #includes still, but I think those complaints are not valid.
Please correct me if I'm mistaken.

> 2. Please consider merging TracePathToRoot bach with Heap implementation.
>     As I understand, the latter is also used for debugging purposes only.

I'll look into it.

> 3. Listing object types in inspector.h looks fragile to me. Is it possible
>     for you to reuse objects-printer.cc?

I agree.  Here's what I propose to make this less fragile: in objects.h,
there is a series of IsXXX() functions.  I'll macro-ize the declaration of
those functions into a list?  That same list is what I need for my
implementation.  I cannot use objects-printer.cc because objects-printer.cc
is for dumping the content of the objects, while I used that object type
list for 2 other purposes: 1. Print only type information, 2. Do macro-ized
type comparisons.  

Is that ok?

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

Reply via email to