http://codereview.chromium.org/2918001/diff/1/21
File src/compiler.cc (right):

http://codereview.chromium.org/2918001/diff/1/21#newcode157
src/compiler.cc:157: static Handle<Object>
CreateScopeInfo(CompilationInfo* info) {
On 2010/07/08 13:23:17, Vitaly wrote:
This should only receive info->scope(). Also consider making it a
static
function in ScopeInfo.

Moved to ScopeInfo

http://codereview.chromium.org/2918001/diff/1/26
File src/frames.cc (right):

http://codereview.chromium.org/2918001/diff/1/26#newcode535
src/frames.cc:535: Handle<Object> scope_info(Heap::empty_fixed_array());
On 2010/07/08 13:23:17, Vitaly wrote:
ScopeInfo::EmptyObject()

Done.

http://codereview.chromium.org/2918001/diff/1/7
File src/heap.cc (right):

http://codereview.chromium.org/2918001/diff/1/7#newcode2322
src/heap.cc:2322: if
(ScopeInfo<>::HasHeapAllocatedLocals(function_info->scope_info()))
On 2010/07/08 13:23:17, Vitaly wrote:
Interesting... This means we could actually flush the code object
while
retaining its scope info. IIRC, Rico added this check later in a
follow-up fix
to his code flushing change, so he should have the numbers on how much
we can
save this way. If the savings are significant, we should consider
whether to
implement this separation now or to file a bug and implement it later.

I will look into it right after this patch is committed.

http://codereview.chromium.org/2918001/diff/1/32
File src/scopeinfo.cc (right):

http://codereview.chromium.org/2918001/diff/1/32#newcode159
src/scopeinfo.cc:159: // - NULL (sentinel)
Agree. Will address that when creating the new class for the heap
object.

On 2010/07/08 13:23:17, Vitaly wrote:
While we're at it, these sentinels look like a waste. We don't seem to
*really*
depend on their presence and that's 12 bytes per JS function.

http://codereview.chromium.org/2918001/diff/1/32#newcode334
src/scopeinfo.cc:334: Object* data = *Factory::NewFixedArray(length,
TENURED);
On 2010/07/08 13:23:17, Vitaly wrote:
Either use Factory and handles or Heap and raw pointers. Factory
methods can
cause garbage collection. Heap methods can return a failure and it
should be
propagated and handled by the callers of ScopeInfo::Serialize(). Since
this
happens during compilation I think using Factory and handles is
preferred. Still
you don't have to use handles in the write functions. Just add
AssertNoAllocation.

Handle<Object> ScopeInfo::Serialize() {
   const int extra_slots = ...;
   int length = ...;
   Handle<Object> data = Factory::New...();
   AssertNoAllocation nogc;
   Object** p0 = GetDataStart(data)
   Object** p = p0;
   // ... Write ...
   return data;
}

Done.

http://codereview.chromium.org/2918001/diff/1/15
File src/scopeinfo.h (right):

http://codereview.chromium.org/2918001/diff/1/15#newcode61
src/scopeinfo.h:61: Object* Serialize();
We agreed that I should refactor the static methods below into instance
methods of the new class (ScopeInfoObject or something like that)
inheriting from FixedArray
On 2010/07/08 13:23:17, Vitaly wrote:
Serialize() returning an Object pointer looks a bit weird. Maybe
ToObject()?

If you agree, please rephrase all other mentions of serialization
here.

http://codereview.chromium.org/2918001/diff/1/15#newcode64
src/scopeinfo.h:64: static Object* SerializeEmpty();
On 2010/07/08 13:23:17, Vitaly wrote:
EmptyObject()?

Done.

http://codereview.chromium.org/2918001/show

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

Reply via email to