I think it'd be nice to (later) create real ScopeInfoObject inheriting from
FixedArray and move the quick accessors (CallsEval, NumberOfStackSlots,
etc.)
there.
-- Vitaly
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) {
This should only receive info->scope(). Also consider making it a static
function in 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());
ScopeInfo::EmptyObject()
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()))
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.
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)
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);
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;
}
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();
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();
EmptyObject()?
http://codereview.chromium.org/2918001/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
