https://codereview.chromium.org/103243005/diff/230001/src/deoptimizer.cc
File src/deoptimizer.cc (right):
https://codereview.chromium.org/103243005/diff/230001/src/deoptimizer.cc#newcode491
src/deoptimizer.cc:491: (*functions)[0]->context()->native_context());
On 2014/01/14 09:50:18, Michael Starzinger wrote:
This seems to assume that all functions in the list come from the same
native
context. Can we be sure this holds? If so we should definitely add an
assert in
that regard into the above loop.
Done.
https://codereview.chromium.org/103243005/diff/230001/src/deoptimizer.cc#newcode3286
src/deoptimizer.cc:3286: switch (map->instance_type()) {
On 2014/01/14 09:50:18, Michael Starzinger wrote:
It would really be nice if we this duplication (the dispatch against
the
instance type and the materialization mechanism below) was avoidable.
I am fine
with factoring this out in a separate CL, but let's leave a TODO here
in that
regard.
Added a TODO for now.
https://codereview.chromium.org/103243005/diff/230001/src/deoptimizer.cc#newcode3381
src/deoptimizer.cc:3381: JSObject::SetElement(array, index,
materialized_objects, NONE, kStrictMode);
On 2014/01/14 09:50:18, Michael Starzinger wrote:
While using a JSArray here to get a "growable array" seems nice, it is
susceptible to a monkey-patched prototype. For example if the array
prototype
contains non-writable elements. If we want to stick with a JSArray, we
need to
switch to an InternalArray.
Replaced with FixedArrays.
https://codereview.chromium.org/103243005/diff/230001/src/isolate.cc
File src/isolate.cc (right):
https://codereview.chromium.org/103243005/diff/230001/src/isolate.cc#newcode308
src/isolate.cc:308: materialized_object_store_->Iterate(v);
On 2014/01/14 09:50:18, Michael Starzinger wrote:
Can we move the pointer to the fixed array into the roots array (i.e.
into
STRONG_ROOT_LIST) instead of explicitly visiting it? We wouldn't need
to
explicitly visit it here, it is directly bound to the heap.
We can keep the other non-heap data (e.g. the FP-mapping list) inside
the
MaterializedObjectStore.
Done.
https://codereview.chromium.org/103243005/diff/230001/src/isolate.h
File src/isolate.h (right):
https://codereview.chromium.org/103243005/diff/230001/src/isolate.h#newcode65
src/isolate.h:65: class MaterializedObjectStore;
On 2014/01/14 09:50:18, Michael Starzinger wrote:
nit: Can we alpha-sort the forward declarations?
Done.
https://codereview.chromium.org/103243005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
