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); On 2012/06/05 13:42:35, danno wrote:
Please use a constant for this. Didn't you add one in your last CL?
Fixed. 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()); On 2012/06/05 13:42:35, danno wrote:
How about add a zone() utility to CompileInfo? This is used a lot in
this file. I'm adding that in the next CL. 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()); On 2012/06/05 13:42:35, danno wrote:
use zone() directly, since the FullCodeGenerator has a direct
reference. This method is static. I'll change this to use the Zone in the CompilationInfo in the next CL. 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, On 2012/06/05 13:42:35, danno wrote:
Make this a shared constant. Also, weird formatting. Put the : on the
next line
and indent accordingly
Fixed. http://codereview.chromium.org/10534006/diff/1/src/hydrogen.cc#newcode5597 src/hydrogen.cc:5597: AddInstruction(HCheckInstanceType::NewIsSpecObject(object, graph()->zone())); On 2012/06/05 13:42:35, danno wrote:
use just zone()
Fixed. http://codereview.chromium.org/10534006/diff/1/src/hydrogen.cc#newcode6313 src/hydrogen.cc:6313: new(isolate()->zone()) HConstant(Handle<Context>(target->context()), On 2012/06/05 13:42:35, danno wrote:
why not just zone()?
Fixed. 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; On 2012/06/05 13:42:35, danno wrote:
Why the const here and not on other version of zone() on other
objects? Fixed. Changed all the zone() methods (except the one in Isolate) to const. 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); On 2012/06/05 13:42:35, danno wrote:
Constant for default argument
Fixed. 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()); On 2012/06/05 13:42:35, danno wrote:
Why do you need allocator?
Not needed. Fixed. 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()), On 2012/06/05 13:42:35, danno wrote:
are you sure of initialization order? either use zone_ everywhere or graph->zone() everywhere.
Fixed. 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()); On 2012/06/05 13:42:35, danno wrote:
just zone
Fixed. http://codereview.chromium.org/10534006/diff/1/src/runtime.cc#newcode6463 src/runtime.cc:6463: ZoneList<int> indices(initial_capacity, isolate->zone()); On 2012/06/05 13:42:35, danno wrote:
just zone
Fixed. 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()) On 2012/06/05 13:42:35, danno wrote:
just zone 2x
Fixed. 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); On 2012/06/05 13:42:35, danno wrote:
Seems weird that you have to pass this into the constructor and again
in
Declare. Cache as member?
I figured not adding a member will save some space. :) Moreover, Declare is called only once in a couple of places, and in all those places I already have a Zone. 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 On 2012/06/05 13:42:35, danno wrote:
I don't understand this code. ForEachNode should take the allocator.
Yes, but we don't have the allocator object in the destructor. http://codereview.chromium.org/10534006/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
