On Thu, Jan 13, 2011 at 19:41, Mark Lam (Palm GBU) <[email protected]> wrote: > 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. >
I didn't looked at the style details. The only thing that always jumps to my eyes are lines >80 chars. Actually, the code review tools runs some other version of linter, so we usually consider only presubmit's errors. >> 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? > As making a list and generating those declarations will save some typing, I think this is a good idea. > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
