Mostly nits, but a few real issues to address.
http://codereview.chromium.org/10534006/diff/1/src/ast.cc File src/ast.cc (right): http://codereview.chromium.org/10534006/diff/1/src/ast.cc#newcode248 src/ast.cc:248: ZoneHashMap table(Literal::Match, 8, allocator); Please use a constant for this. Didn't you add one in your last CL? http://codereview.chromium.org/10534006/diff/1/src/compiler.cc File src/compiler.cc (right): http://codereview.chromium.org/10534006/diff/1/src/compiler.cc#newcode351 src/compiler.cc:351: info->isolate()->zone()); How about add a zone() utility to CompileInfo? This is used a lot in this file. http://codereview.chromium.org/10534006/diff/1/src/full-codegen.cc File src/full-codegen.cc (right): http://codereview.chromium.org/10534006/diff/1/src/full-codegen.cc#newcode306 src/full-codegen.cc:306: FullCodeGenerator cgen(&masm, info, isolate->zone()); use zone() directly, since the FullCodeGenerator has a direct reference. http://codereview.chromium.org/10534006/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/10534006/diff/1/src/hydrogen.cc#newcode3138 src/hydrogen.cc:3138: explicit BoundsCheckTable(Zone* zone) : ZoneHashMap(BoundsCheckKeyMatch, 8, Make this a shared constant. Also, weird formatting. Put the : on the next line and indent accordingly http://codereview.chromium.org/10534006/diff/1/src/hydrogen.cc#newcode5597 src/hydrogen.cc:5597: AddInstruction(HCheckInstanceType::NewIsSpecObject(object, graph()->zone())); use just zone() http://codereview.chromium.org/10534006/diff/1/src/hydrogen.cc#newcode6313 src/hydrogen.cc:6313: new(isolate()->zone()) HConstant(Handle<Context>(target->context()), why not just zone()? http://codereview.chromium.org/10534006/diff/1/src/hydrogen.h File src/hydrogen.h (right): http://codereview.chromium.org/10534006/diff/1/src/hydrogen.h#newcode617 src/hydrogen.h:617: inline Zone* zone() const; Why the const here and not on other version of zone() on other objects? http://codereview.chromium.org/10534006/diff/1/src/hydrogen.h#newcode1154 src/hydrogen.h:1154: Zone* zone() const { return zone_; } Why the const here and not on other version of zone() on other objects? http://codereview.chromium.org/10534006/diff/1/src/hydrogen.h#newcode1186 src/hydrogen.h:1186: Zone* AstContext::zone() const { return owner_->zone(); } Why the const here and not on other version of zone() on other objects? http://codereview.chromium.org/10534006/diff/1/src/interface.cc File src/interface.cc (right): http://codereview.chromium.org/10534006/diff/1/src/interface.cc#newcode93 src/interface.cc:93: *map = new ZoneHashMap(Match, 8, allocator); Constant for default argument http://codereview.chromium.org/10534006/diff/1/src/jsregexp.cc File src/jsregexp.cc (right): http://codereview.chromium.org/10534006/diff/1/src/jsregexp.cc#newcode4374 src/jsregexp.cc:4374: ZoneAllocationPolicy allocator(that->zone()); Why do you need allocator? http://codereview.chromium.org/10534006/diff/1/src/lithium-allocator.cc File src/lithium-allocator.cc (right): http://codereview.chromium.org/10534006/diff/1/src/lithium-allocator.cc#newcode537 src/lithium-allocator.cc:537: live_ranges_(num_values * 2, graph->zone()), are you sure of initialization order? either use zone_ everywhere or graph->zone() everywhere. http://codereview.chromium.org/10534006/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/10534006/diff/1/src/runtime.cc#newcode3741 src/runtime.cc:3741: ZoneList<int> offsets(8, isolate->zone()); just zone http://codereview.chromium.org/10534006/diff/1/src/runtime.cc#newcode6463 src/runtime.cc:6463: ZoneList<int> indices(initial_capacity, isolate->zone()); just zone http://codereview.chromium.org/10534006/diff/1/src/safepoint-table.cc File src/safepoint-table.cc (right): http://codereview.chromium.org/10534006/diff/1/src/safepoint-table.cc#newcode142 src/safepoint-table.cc:142: ? new(assembler->zone()) ZoneList<int>(4, assembler->zone()) just zone 2x http://codereview.chromium.org/10534006/diff/1/src/scopes.h File src/scopes.h (right): http://codereview.chromium.org/10534006/diff/1/src/scopes.h#newcode43 src/scopes.h:43: explicit VariableMap(Zone* zone); Seems weird that you have to pass this into the constructor and again in Declare. Cache as member? http://codereview.chromium.org/10534006/diff/1/src/splay-tree-inl.h File src/splay-tree-inl.h (right): http://codereview.chromium.org/10534006/diff/1/src/splay-tree-inl.h#newcode39 src/splay-tree-inl.h:39: // Pre-allocate some space for tiny trees. Since we don't have an I don't understand this code. ForEachNode should take the allocator. http://codereview.chromium.org/10534006/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
