You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Weldon Washburn <we...@gmail.com> on 2006/08/28 17:32:52 UTC

[DRLVM][VM] set_hash_bits() in vmcore/src/thread/mon_enter_exit.cpp -- is it a bug or a feature?

While porting MMTk to harmony/drlvm, I hit an integration problem.  It
could even be a bug.  set_hash_bits() assumes the least significant
bit is zero.  Assuming that the LSB can be "owned" by the garbage
collector for its purposes, set_hash_bits() will fail if the GC sets
this bit to one.  Somehow I think the code should read the target
location, create the intended bit pattern before attempting to do the
atomic compare and swap.  Currently the code assume the target CAS
location holds zero.

SInce I am working only in single thread right now, I hacked around
the problem with the below.  Thoughts?


C:\t_harmony\drlvm\trunk\vm\vmcore\src\thread>svn diff mon_enter_exit.cpp
Index: mon_enter_exit.cpp
===================================================================
--- mon_enter_exit.cpp  (revision 425482)
+++ mon_enter_exit.cpp  (working copy)
@@ -368,7 +368,12 @@
         hb = (23 & HASH_MASK);  // NO hash = zero allowed, thus hard map hb = 0
 to a fixed prime number

     // don't care if the cmpxchg fails -- just means someone else already set t
he hash
-    port_atomic_cas8(P_HASH_CONTENTION(p_obj),hb, 0);
+    //port_atomic_cas8(P_HASH_CONTENTION(p_obj),hb, 0);
+    unsigned char lsb = *P_HASH_CONTENTION(p_obj);
+    lsb = lsb & 0x01;  //wjw need to keep the LSB, its used by MMTk Garbage Col
lector
+    hb = hb | lsb;
+    if ( (*P_HASH_CONTENTION(p_obj) & HASH_MASK) == 0 ) // wjw non-atomic hack
for now
+        *P_HASH_CONTENTION(p_obj) = hb;
 }




-- 
Weldon Washburn
Intel Middleware Products Division

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Re: [DRLVM][VM] set_hash_bits() in vmcore/src/thread/mon_enter_exit.cpp -- is it a bug or a feature?

Posted by Weldon Washburn <we...@gmail.com>.
On 9/1/06, Artem Aliev <ar...@gmail.com> wrote:
>
> Guys,
>
> The problem looks much broader then just a hash code placing.
> There is no common place that describe object header format and object
> header sharing protocol.


Yes, I noticed the same thing.  This was the original motivation for
starting these email threads on the topic.  Once we as a group have a good
handle on what functionality the header bits contains, we can then figure
out what flexibility needs to be built into the object model.

There are 3 components that use object header together.
> VMcore -- store VT pointer and hashcode
> ThreadManager -- update lockword
> GC -- tries to find unused bits for its own purpose
>          Different GC algorithms use different unused bits:
>           2-bits from aligned VT  pointer,
>           move hashcode to the synthetic field and use last byte
>           truncate hashcode
>
> So by moving hash_code() handling to GC and drop VT bit usage
> scenario, you simplify the problem to GC- TM interaction but not solve
> it.


>From the other email thread I started on the subject, Robin Garner and I
came to the conclusion that giving one byte of object header to the GC to do
Object.hashCode() plus whatever else the GC wants to do should be sufficient
for the foreseeable future.  I suspect the MMTk community would think the
same way.  This leaves 3 bytes on a 32-bit machine for threading (thread
lock ID, recursion count, etc.) and 7 bytes on a 64-bit machine (unless
header compression is a requirement.)

We still need the protocol.


Please tell me if the above covers the need for a protocol.

The other problem is the 64bit platforms.(for example EM64T).
> It looks natural to have 64bit VT and lockword on such platforms
>
> So we need not only sharing protocol bu also a more flexible object
> layout.


Yes, it would be nice to have an "expando" object layout.  From what I
recall we did a header size trade-off study using ORP about 5 years ago.  It
was a couple of percent performance delta when adding/subtracting 4 bytes
from the object header.  Going from 8-byte to 4-byte header is certainly
possible but probably makes the JVM real fragile since there is such
pressure on the precious header bits.  Going to 12 bytes costs a couple of
percentage points so there needs to be some compelling reason to burn the
extra bits -- I just don't know any compelling reasons.

Thanks
> Artem
>
> PS:
> At this time we have following 64bit object header fromat
> | ------------------ 32 bit -------------------|
>        Virtual Method Table pointer
> |------------- 22bit -------|------10bit-----|
> Monitor lockword bits         ^unused/hashcode/gc_bits
>
>
>
> --
> Weldon Washburn
> Intel Middleware Products Division

Re: [DRLVM][VM] set_hash_bits() in vmcore/src/thread/mon_enter_exit.cpp -- is it a bug or a feature?

Posted by Artem Aliev <ar...@gmail.com>.
Guys,

The problem looks much broader then just a hash code placing.
There is no common place that describe object header format and object
header sharing protocol.
There are 3 components that use object header together.
VMcore -- store VT pointer and hashcode
ThreadManager -- update lockword
GC -- tries to find unused bits for its own purpose
          Different GC algorithms use different unused bits:
           2-bits from aligned VT  pointer,
           move hashcode to the synthetic field and use last byte
           truncate hashcode

So by moving hash_code() handling to GC and drop VT bit usage
scenario, you simplify the problem to GC- TM interaction but not solve
it.

We still need the protocol.

The other problem is the 64bit platforms.(for example EM64T).
It looks natural to have 64bit VT and lockword on such platforms

So we need not only sharing protocol bu also a more flexible object layout.

Thanks
Artem

PS:
At this time we have following 64bit object header fromat
| ------------------ 32 bit -------------------|
        Virtual Method Table pointer
|------------- 22bit -------|------10bit-----|
  Monitor lockword bits         ^unused/hashcode/gc_bits


On 8/29/06, Ivan Volosyuk <iv...@gmail.com> wrote:
> This is the GC_V4 specifics. It uses two bits of least significant
> byte of the status word during GC, but it always return zeros to this
> bits after GC.
>
> Here is the atomic way to do the changes:
>   do {
>      unsigned char lsb = *P_HASH_CONTENTION(p_obj);
>      unsigned char new_lsb = lsb | hb;
>      unsigned char uptodate_lsb =
>           port_atomic_cas8(P_HASH_CONTENTION(p_obj), new_lsb, lsb);
>   while (lsb != uptodate_lsb);
>
> BTW, atomic operations is only needed if GC wants to update those two
> bits during mutator work. Otherwise, non-atomic operations is ok, as
> the same value can be safely written to the same address by multiple
> threads. This is one pros for moving this code to GC.
>
> --
> Ivan
>
> On 8/28/06, Weldon Washburn <we...@gmail.com> wrote:
> > While porting MMTk to harmony/drlvm, I hit an integration problem.  It
> > could even be a bug.  set_hash_bits() assumes the least significant
> > bit is zero.  Assuming that the LSB can be "owned" by the garbage
> > collector for its purposes, set_hash_bits() will fail if the GC sets
> > this bit to one.  Somehow I think the code should read the target
> > location, create the intended bit pattern before attempting to do the
> > atomic compare and swap.  Currently the code assume the target CAS
> > location holds zero.
> >
> > SInce I am working only in single thread right now, I hacked around
> > the problem with the below.  Thoughts?
> >
> >
> > C:\t_harmony\drlvm\trunk\vm\vmcore\src\thread>svn diff mon_enter_exit.cpp
> > Index: mon_enter_exit.cpp
> > ===================================================================
> > --- mon_enter_exit.cpp  (revision 425482)
> > +++ mon_enter_exit.cpp  (working copy)
> > @@ -368,7 +368,12 @@
> >          hb = (23 & HASH_MASK);  // NO hash = zero allowed, thus hard map hb = 0
> >  to a fixed prime number
> >
> >      // don't care if the cmpxchg fails -- just means someone else already set t
> > he hash
> > -    port_atomic_cas8(P_HASH_CONTENTION(p_obj),hb, 0);
> > +    //port_atomic_cas8(P_HASH_CONTENTION(p_obj),hb, 0);
> > +    unsigned char lsb = *P_HASH_CONTENTION(p_obj);
> > +    lsb = lsb & 0x01;  //wjw need to keep the LSB, its used by MMTk Garbage Col
> > lector
> > +    hb = hb | lsb;
> > +    if ( (*P_HASH_CONTENTION(p_obj) & HASH_MASK) == 0 ) // wjw non-atomic hack
> > for now
> > +        *P_HASH_CONTENTION(p_obj) = hb;
> >  }
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Re: [DRLVM][VM] set_hash_bits() in vmcore/src/thread/mon_enter_exit.cpp -- is it a bug or a feature?

Posted by Ivan Volosyuk <iv...@gmail.com>.
This is the GC_V4 specifics. It uses two bits of least significant
byte of the status word during GC, but it always return zeros to this
bits after GC.

Here is the atomic way to do the changes:
  do {
     unsigned char lsb = *P_HASH_CONTENTION(p_obj);
     unsigned char new_lsb = lsb | hb;
     unsigned char uptodate_lsb =
          port_atomic_cas8(P_HASH_CONTENTION(p_obj), new_lsb, lsb);
  while (lsb != uptodate_lsb);

BTW, atomic operations is only needed if GC wants to update those two
bits during mutator work. Otherwise, non-atomic operations is ok, as
the same value can be safely written to the same address by multiple
threads. This is one pros for moving this code to GC.

--
Ivan

On 8/28/06, Weldon Washburn <we...@gmail.com> wrote:
> While porting MMTk to harmony/drlvm, I hit an integration problem.  It
> could even be a bug.  set_hash_bits() assumes the least significant
> bit is zero.  Assuming that the LSB can be "owned" by the garbage
> collector for its purposes, set_hash_bits() will fail if the GC sets
> this bit to one.  Somehow I think the code should read the target
> location, create the intended bit pattern before attempting to do the
> atomic compare and swap.  Currently the code assume the target CAS
> location holds zero.
>
> SInce I am working only in single thread right now, I hacked around
> the problem with the below.  Thoughts?
>
>
> C:\t_harmony\drlvm\trunk\vm\vmcore\src\thread>svn diff mon_enter_exit.cpp
> Index: mon_enter_exit.cpp
> ===================================================================
> --- mon_enter_exit.cpp  (revision 425482)
> +++ mon_enter_exit.cpp  (working copy)
> @@ -368,7 +368,12 @@
>          hb = (23 & HASH_MASK);  // NO hash = zero allowed, thus hard map hb = 0
>  to a fixed prime number
>
>      // don't care if the cmpxchg fails -- just means someone else already set t
> he hash
> -    port_atomic_cas8(P_HASH_CONTENTION(p_obj),hb, 0);
> +    //port_atomic_cas8(P_HASH_CONTENTION(p_obj),hb, 0);
> +    unsigned char lsb = *P_HASH_CONTENTION(p_obj);
> +    lsb = lsb & 0x01;  //wjw need to keep the LSB, its used by MMTk Garbage Col
> lector
> +    hb = hb | lsb;
> +    if ( (*P_HASH_CONTENTION(p_obj) & HASH_MASK) == 0 ) // wjw non-atomic hack
> for now
> +        *P_HASH_CONTENTION(p_obj) = hb;
>  }
>

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org