Filter old to new references when moving parts of an object.

Filter old space and large object space to new space references when moving parts of a FixedArray.


BUG=v8:2452
Committed: https://code.google.com/p/v8/source/detail?r=13343

[Michael Starzinger, 2013-01-08 13:13:00]

In general this is going the right way, I just have one high level comment about
the interface. Also we need to make sure that the filtering doesn't regress
array operations on small or medium-sized arrays (not in large-object space).

https://codereview.chromium.org/11737006/diff/4001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/11737006/diff/4001/src/heap.h#newcode528
src/heap.h:528: void MoveElements(FixedArray* dst, int dst_index, FixedArray*
src,
This method needs a comment explaining it's semantics. Also I would move it down
to the other FixedArray methods (e.g. somewhere around CopyFixedArray).

The assumption is that whenever we move values in an object that "src == dst"
holds. So I think it makes more sence to have an API that just takes one object
pointer, two offsets into it and one length. This also makes further
optimizations easier. WDYT?

[Michael Starzinger, 2013-01-08 13:46:31]

One additional comment for the record.

https://codereview.chromium.org/11737006/diff/4001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/11737006/diff/4001/src/heap.cc#newcode690
src/heap.cc:690: if (!InNewSpace(src) && InNewSpace(dst_objects[i])) {
As discussed offline, this second loop is no longer cache-local because it
interleaves access to the store-buffer with access to the destination. I am fine
with landing this for now if it doesn't regress until we optimize the
store-buffer access, but please leave a TODO comment here.

Also the InNewSpace(src) check can be hoisted out of the loop, because I am not
sure GCC is smart enough to do that.

[Hannes Payer (out of office), 2013-01-09 09:32:46]

https://codereview.chromium.org/11737006/diff/4001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/11737006/diff/4001/src/heap.cc#newcode690
src/heap.cc:690: if (!InNewSpace(src) && InNewSpace(dst_objects[i])) {
On 2013/01/08 13:46:32, Michael Starzinger wrote:
> As discussed offline, this second loop is no longer cache-local because it
> interleaves access to the store-buffer with access to the destination. I am
fine
> with landing this for now if it doesn't regress until we optimize the
> store-buffer access, but please leave a TODO comment here.

I will submit an optimization in a follow-up CL.

> Also the InNewSpace(src) check can be hoisted out of the loop, because I am
not
> sure GCC is smart enough to do that.
Done.

https://codereview.chromium.org/11737006/diff/4001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/11737006/diff/4001/src/heap.h#newcode528
src/heap.h:528: void MoveElements(FixedArray* dst, int dst_index, FixedArray*
src,
On 2013/01/08 13:13:00, Michael Starzinger wrote:
> This method needs a comment explaining it's semantics. Also I would move it
down
> to the other FixedArray methods (e.g. somewhere around CopyFixedArray).
> 
> The assumption is that whenever we move values in an object that "src == dst"
> holds. So I think it makes more sence to have an API that just takes one
object
> pointer, two offsets into it and one length. This also makes further
> optimizations easier. WDYT?

Done.

[Michael Starzinger, 2013-01-09 12:58:09]

LGTM.