http://codereview.chromium.org/10103035/diff/23027/src/compiler.cc File src/compiler.cc (right):
http://codereview.chromium.org/10103035/diff/23027/src/compiler.cc#newcode603 src/compiler.cc:603: static void AddToOptimizedCodeMap(Handle<SharedFunctionInfo> shared, On 2012/05/23 11:16:29, Michael Starzinger wrote:
Can we move this into the the SharedFunctionInfo class so that it is
coupled
with the corresponding search method?
Done. http://codereview.chromium.org/10103035/diff/23027/src/compiler.cc#newcode609 src/compiler.cc:609: const int kEntryLength = 3; On 2012/05/23 11:16:29, Michael Starzinger wrote:
Moving these constants into SharedFunctionInfo and reusing them in the
search
method seems reasonable as well.
Done. http://codereview.chromium.org/10103035/diff/23027/src/compiler.cc#newcode626 src/compiler.cc:626: for (int i = 0; i < old_length; i += kEntryLength) { On 2012/05/23 11:16:29, Michael Starzinger wrote:
Can we use FixedArray::CopyTo or even a handlified version of FixedArray::CopySize or a similar method to do the copying?
Done. http://codereview.chromium.org/10103035/diff/23027/src/factory.cc File src/factory.cc (right): http://codereview.chromium.org/10103035/diff/23027/src/factory.cc#newcode561 src/factory.cc:561: FixedArray::cast(code_map->get(index + 1)); On 2012/05/23 11:16:29, Michael Starzinger wrote:
It would be nice to have constants for these offsets (i.e. -1, 0 and
+1) in
SharedFunctionInfo.
Agree. Though I'd have the constants relative to the entry start so that it does not get confusing: 0 = context, 1 = code, 2 = literals SearchOptimizedCodeMap would have to accordingly return the entry start instead of the Code* http://codereview.chromium.org/10103035/diff/23027/src/factory.cc#newcode562 src/factory.cc:562: ASSERT(cached_literals != NULL); On 2012/05/23 11:16:29, Michael Starzinger wrote:
Better add an ASSERT that the global context stored in the cached
literals array
still matches.
Done. http://codereview.chromium.org/10103035/diff/23027/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/10103035/diff/23027/src/ia32/code-stubs-ia32.cc#newcode130 src/ia32/code-stubs-ia32.cc:130: const int kEntryLength = 3; On 2012/05/23 11:16:29, Michael Starzinger wrote:
Again, having this constant in SharedFunctionInfo seems safer.
Done. http://codereview.chromium.org/10103035/diff/23027/src/ia32/code-stubs-ia32.cc#newcode7146 src/ia32/code-stubs-ia32.cc:7146: { ecx, edx, ebx, EMIT_REMEMBERED_SET}, On 2012/05/23 11:16:29, Michael Starzinger wrote:
Use the REG macro as well.
Done. http://codereview.chromium.org/10103035/diff/23027/src/ia32/deoptimizer-ia32.cc File src/ia32/deoptimizer-ia32.cc (right): http://codereview.chromium.org/10103035/diff/23027/src/ia32/deoptimizer-ia32.cc#newcode122 src/ia32/deoptimizer-ia32.cc:122: function->shared()->set_optimized_code_map(Smi::FromInt(0)); On 2012/05/23 11:16:29, Michael Starzinger wrote:
I think it would make sense to have shared->ClearOptimizedCodeMap() as
a helper. Done. http://codereview.chromium.org/10103035/diff/23027/src/mark-compact.cc File src/mark-compact.cc (right): http://codereview.chromium.org/10103035/diff/23027/src/mark-compact.cc#newcode1305 src/mark-compact.cc:1305: reinterpret_cast<SharedFunctionInfo*>(object)->BeforeVisitingPointers(); On 2012/05/23 11:16:29, Michael Starzinger wrote:
We should be able to use SharedFunctionInfo::cast() with the new GC.
Done. http://codereview.chromium.org/10103035/diff/23027/src/objects.h File src/objects.h (right): http://codereview.chromium.org/10103035/diff/23027/src/objects.h#newcode5164 src/objects.h:5164: // i - 1 is the context, position i the code, and i + 1 the literals array. On 2012/05/23 11:16:29, Michael Starzinger wrote:
Can we add a comment about what happens when no entry is found?
Done. http://codereview.chromium.org/10103035/diff/23027/src/objects.h#newcode5274 src/objects.h:5274: // Invoked before pointers in SharedFunctionInfo are being marked. On 2012/05/23 11:16:29, Michael Starzinger wrote:
It would be nice to also have a short comment what this method does on
top of
when it is being called.
Done. http://codereview.chromium.org/10103035/diff/23027/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/10103035/diff/23027/src/runtime.cc#newcode8255 src/runtime.cc:8255: function->shared()->set_optimized_code_map(Smi::FromInt(0)); On 2012/05/23 11:16:29, Michael Starzinger wrote:
It seems we can hoist that out of the condition.
Done. http://codereview.chromium.org/10103035/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
