On 2015/02/04 10:49:42, Erik Corry wrote:
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?
It is not an issue for the sweeper thread, since the size of the different
WeakCells is the same.
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.
