[m-rev.] for review: threadsafe version_arrays in Java

[Paul Bone]

On Mon, Apr 19, 2010 at 12:17:15PM +1000, Peter Wang wrote:
> On 19 April 2010 12:00, Paul Bone <pbone at csse.unimelb.edu.au> wrote:
> >> @@ -678,6 +731,23 @@ ML_va_get(ML_va_ptr VA, MR_Integer I, MR_Word *Xptr)
> >>  }
> >>
> >>  int
> >> +ML_va_set_dolock(ML_va_ptr VA0, MR_Integer I, MR_Word X, ML_va_ptr *VAptr)
> >> +{
> >> +#ifdef MR_THREAD_SAFE
> >> +    MercuryLock *lock = VA0->lock;
> >> +#endif
> >> +    int         ret;
> >> +
> >> +    ML_maybe_lock(lock);
> >> +
> >> +    ret = ML_va_set(VA0, I, X, VAptr);
> >> +
> >> +    ML_maybe_unlock(lock);
> >> +
> >> +    return ret;
> >> +}
> >> +
> >> +static int
> >>  ML_va_set(ML_va_ptr VA0, MR_Integer I, MR_Word X, ML_va_ptr *VAptr)
> >>  {
> >>      ML_va_ptr VA1;
> >> @@ -691,6 +761,9 @@ ML_va_set(ML_va_ptr VA0, MR_Integer I, MR_Word X,
> >> ML_va_ptr *VAptr)
> >>          VA1->index      = -1;
> >>          VA1->value      = (MR_Word) NULL;
> >>          VA1->rest.array = VA0->rest.array;
> >> +#ifdef MR_THREAD_SAFE
> >> +        VA1->lock       = VA0->lock;
> >> +#endif
> >>
> >>          VA0->index     = I;
> >>          VA0->value     = VA0->rest.array->elements[I];
> >
> > I'm concerned about the copying of this lock.  Is it a copy or do VA0->lock and
> > VA1->lock both refer to the same lock.  VA0->lock is currently locked and will
> > be unlocked by ML_va_set_dolock.  Will this unlock VA1->lock?  (which it
> > should).
> 
> Here we create VA1 as the latest version of the array, VA0 being the
> latest version before that.  Both lead to the same underlying array,
> so they need to share the same lock.  So ML_va_set_dolock will indeed
> unlock it.
> 

But do you know that this is a reference to the same lock (pointer to a lock)
or just a copy of the lock?

It might be just me, but this would be clearer with a comment.

Thanks.

> Thanks for looking.

NP.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 489 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20100419/ad089a66/attachment.sig>