Thanks for review.

The two different WeakCell maps are identical from the point of view of the
sweeper, so I think the data race makes no difference. If we have to check the state of the sweeper, then that would make this approach much less attractive.


https://codereview.chromium.org/893073006/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/893073006/diff/1/src/hydrogen-instructions.h#newcode6220
src/hydrogen-instructions.h:6220: static HObjectAccess
ForWeakCellValue() {
On 2015/02/04 09:35:04, ulan wrote:
On 2015/02/03 17:42:53, Erik Corry http://Chromium.org wrote:
> I suspect some or perhaps all of the uses of this are for checks and
should
not
> trigger the read barrier.

Yesterday I landed a change in StoreGlobal hydrogen stub that loads
and uses
weak value.

OK, we probably need both.

https://codereview.chromium.org/893073006/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

https://codereview.chromium.org/893073006/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode2995
src/ia32/lithium-codegen-ia32.cc:2995: __ mov(result,
FieldOperand(object, HeapObject::kMapOffset));
On 2015/02/04 09:35:04, ulan wrote:
__ mov(FieldOperand(object, HeapObject::kMapOffset), result);

This is intended to be a store, not a load.

https://codereview.chromium.org/893073006/diff/1/src/ic/ia32/ic-compiler-ia32.cc
File src/ic/ia32/ic-compiler-ia32.cc (right):

https://codereview.chromium.org/893073006/diff/1/src/ic/ia32/ic-compiler-ia32.cc#newcode117
src/ic/ia32/ic-compiler-ia32.cc:117: Handle<WeakCell> cell =
Map::WeakCellForMap(transitioned_maps->at(i));
On 2015/02/04 09:35:04, ulan wrote:
On 2015/02/03 17:42:53, Erik Corry http://Chromium.org wrote:
> I think perhaps here we should trigger the read barrier on the first
cell
(yeah
> there are two variables called cell in this function!) to show that
it is in
use
> and should not be cleared on GC. What do you think?

Do you mean read barrier on receiver_map? I think we don't need it. If
the
compare succeeds than we already have the receiver map in register, so
it must
be alive. If the compare fails, then we have no information and we
didn't "use"
the map.

I agree we don't need it for correctness.  The question is whether it
could be useful for tracking whether the map was recently used, so that
we can extend its life in GC even if there are no live objects using it
at the next GC.  But I'm not exactly sure what's going on here: Does
this count as a recent use that should prevent weak map clearing at the
next GC?

https://codereview.chromium.org/893073006/

--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to