You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openwebbeans.apache.org by David Blevins <da...@gmail.com> on 2013/05/11 22:59:44 UTC
AnnotatedTypeImpl and thread safety
There was some doubt on IRC as to if AnnotatedType should be thread safe as well as some comments that eliminating the use of AnnotatedType at runtime would be a possible solution.
We certainly can reduce or eliminate our use of AnnotatedType, but noticed the BeanManager API gives us no choice as it allows people to create and use AnnotatedType at runtime.
final BeanManager beanManager = getBeanManager();
final AnnotatedType<Colors> annotatedType = beanManager.createAnnotatedType(Colors.class);
for (int i = 0; i < threads; i++)
{
new Runner(startingLine, startingPistol, exceptions, finishLine, annotatedType)
{
@Override
public void doit()
{
beanManager.createInjectionTarget(annotatedType);
}
}.start();
}
That being said, I'll see if I can reduce the memory requirement of the fix. Speed-wise, it's good. FutureTask is fast.
-David
Re: AnnotatedTypeImpl and thread safety
Posted by Romain Manni-Bucau <rm...@gmail.com>.
just checked impl (in java7) and it sounds fine for me, thanks David!
*Romain Manni-Bucau*
*Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
*Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
*LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
*Github: https://github.com/rmannibucau*
2013/5/12 David Blevins <da...@gmail.com>
> We should be good. UnmodifiableSet + iterator are just a wrappers and
> HashSet is thread-safe for read-only usage. They're now populated in the
> constructor as local variables, so outside the constructor no one has a
> reference to them.
>
> If we revert back to the code where the hashsets are exposed, then we
> should probably use a CopyOnWriteArraySet as you suggest.
>
>
> -David
>
> On May 12, 2013, at 11:36 AM, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
> > If you guarantee me unmodifiableSet itertors are thread safe i'm happy
> > otherwise please use a copyonwriteset then it'll not say anything more
> > Le 12 mai 2013 20:15, "David Blevins" <da...@gmail.com> a écrit
> :
> >
> >> Looked into that this morning.
> >>
> >> It occurred to me that would definitely make it thread-safe, but alone
> >> doesn't give us the right logic for AnnotatedType.getMethods(), etc.
> which
> >> are supposed to return a complete list not a list being updated. So
> we'd
> >> still need to block getMethods() calls until initialization is complete.
> >>
> >> Updated the code again. Have a look and let me know if you see any more
> >> holes :) The lists not being thread safe was definitely accurate.
> >>
> >>
> >> -David
> >>
> >> On May 11, 2013, at 11:49 PM, Romain Manni-Bucau <rmannibucau@gmail.com
> >
> >> wrote:
> >>
> >>> I keep my comment/explanation on it, if you want it thread safe at
> >> runtime
> >>> convert it to sthg thread safe...copyonwriteXXX structures will be
> >> enough.
> >>> We dont need what you did IMO
> >>> Le 12 mai 2013 00:04, "David Blevins" <da...@gmail.com> a
> écrit
> >> :
> >>>
> >>>>
> >>>> On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> That being said, I'll see if I can reduce the memory requirement of
> the
> >>>> fix.
> >>>>
> >>>> Done. Managed to reduce the memory requirement of the original code
> for
> >>>> the lazy case as well. Basically two object references lighter (128
> >> bits).
> >>>>
> >>>> Thread-safe, lighter and slightly more immutable.
> >>>>
> >>>>
> >>>> -David
> >>>>
> >>>>
> >>
> >>
>
>
Re: AnnotatedTypeImpl and thread safety
Posted by David Blevins <da...@gmail.com>.
We should be good. UnmodifiableSet + iterator are just a wrappers and HashSet is thread-safe for read-only usage. They're now populated in the constructor as local variables, so outside the constructor no one has a reference to them.
If we revert back to the code where the hashsets are exposed, then we should probably use a CopyOnWriteArraySet as you suggest.
-David
On May 12, 2013, at 11:36 AM, Romain Manni-Bucau <rm...@gmail.com> wrote:
> If you guarantee me unmodifiableSet itertors are thread safe i'm happy
> otherwise please use a copyonwriteset then it'll not say anything more
> Le 12 mai 2013 20:15, "David Blevins" <da...@gmail.com> a écrit :
>
>> Looked into that this morning.
>>
>> It occurred to me that would definitely make it thread-safe, but alone
>> doesn't give us the right logic for AnnotatedType.getMethods(), etc. which
>> are supposed to return a complete list not a list being updated. So we'd
>> still need to block getMethods() calls until initialization is complete.
>>
>> Updated the code again. Have a look and let me know if you see any more
>> holes :) The lists not being thread safe was definitely accurate.
>>
>>
>> -David
>>
>> On May 11, 2013, at 11:49 PM, Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>>
>>> I keep my comment/explanation on it, if you want it thread safe at
>> runtime
>>> convert it to sthg thread safe...copyonwriteXXX structures will be
>> enough.
>>> We dont need what you did IMO
>>> Le 12 mai 2013 00:04, "David Blevins" <da...@gmail.com> a écrit
>> :
>>>
>>>>
>>>> On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com>
>>>> wrote:
>>>>
>>>>> That being said, I'll see if I can reduce the memory requirement of the
>>>> fix.
>>>>
>>>> Done. Managed to reduce the memory requirement of the original code for
>>>> the lazy case as well. Basically two object references lighter (128
>> bits).
>>>>
>>>> Thread-safe, lighter and slightly more immutable.
>>>>
>>>>
>>>> -David
>>>>
>>>>
>>
>>
Re: AnnotatedTypeImpl and thread safety
Posted by Romain Manni-Bucau <rm...@gmail.com>.
If you guarantee me unmodifiableSet itertors are thread safe i'm happy
otherwise please use a copyonwriteset then it'll not say anything more
Le 12 mai 2013 20:15, "David Blevins" <da...@gmail.com> a écrit :
> Looked into that this morning.
>
> It occurred to me that would definitely make it thread-safe, but alone
> doesn't give us the right logic for AnnotatedType.getMethods(), etc. which
> are supposed to return a complete list not a list being updated. So we'd
> still need to block getMethods() calls until initialization is complete.
>
> Updated the code again. Have a look and let me know if you see any more
> holes :) The lists not being thread safe was definitely accurate.
>
>
> -David
>
> On May 11, 2013, at 11:49 PM, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
> > I keep my comment/explanation on it, if you want it thread safe at
> runtime
> > convert it to sthg thread safe...copyonwriteXXX structures will be
> enough.
> > We dont need what you did IMO
> > Le 12 mai 2013 00:04, "David Blevins" <da...@gmail.com> a écrit
> :
> >
> >>
> >> On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com>
> >> wrote:
> >>
> >>> That being said, I'll see if I can reduce the memory requirement of the
> >> fix.
> >>
> >> Done. Managed to reduce the memory requirement of the original code for
> >> the lazy case as well. Basically two object references lighter (128
> bits).
> >>
> >> Thread-safe, lighter and slightly more immutable.
> >>
> >>
> >> -David
> >>
> >>
>
>
Re: AnnotatedTypeImpl and thread safety
Posted by David Blevins <da...@gmail.com>.
Looked into that this morning.
It occurred to me that would definitely make it thread-safe, but alone doesn't give us the right logic for AnnotatedType.getMethods(), etc. which are supposed to return a complete list not a list being updated. So we'd still need to block getMethods() calls until initialization is complete.
Updated the code again. Have a look and let me know if you see any more holes :) The lists not being thread safe was definitely accurate.
-David
On May 11, 2013, at 11:49 PM, Romain Manni-Bucau <rm...@gmail.com> wrote:
> I keep my comment/explanation on it, if you want it thread safe at runtime
> convert it to sthg thread safe...copyonwriteXXX structures will be enough.
> We dont need what you did IMO
> Le 12 mai 2013 00:04, "David Blevins" <da...@gmail.com> a écrit :
>
>>
>> On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com>
>> wrote:
>>
>>> That being said, I'll see if I can reduce the memory requirement of the
>> fix.
>>
>> Done. Managed to reduce the memory requirement of the original code for
>> the lazy case as well. Basically two object references lighter (128 bits).
>>
>> Thread-safe, lighter and slightly more immutable.
>>
>>
>> -David
>>
>>
Re: AnnotatedTypeImpl and thread safety
Posted by Romain Manni-Bucau <rm...@gmail.com>.
I keep my comment/explanation on it, if you want it thread safe at runtime
convert it to sthg thread safe...copyonwriteXXX structures will be enough.
We dont need what you did IMO
Le 12 mai 2013 00:04, "David Blevins" <da...@gmail.com> a écrit :
>
> On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com>
> wrote:
>
> > That being said, I'll see if I can reduce the memory requirement of the
> fix.
>
> Done. Managed to reduce the memory requirement of the original code for
> the lazy case as well. Basically two object references lighter (128 bits).
>
> Thread-safe, lighter and slightly more immutable.
>
>
> -David
>
>
Re: AnnotatedTypeImpl and thread safety
Posted by David Blevins <da...@gmail.com>.
Further updated so that the hashsets are not modified outside the AnnotatedTypeImpl.State constructor.
Dug around and the only usage of the AnnotatedTypeImpl.addAnnotatedMethod(..) and similar methods was if someone attempted to construct an AnnotatedFieldImpl, etc. and not pass in a declaringType. I looked through all the code usages, ran all the tests, ran the TCK in OWB and in TomEE and this never happened.
If anyone sees any issues with it, let me know.
-David
On May 11, 2013, at 3:04 PM, David Blevins <da...@gmail.com> wrote:
>
> On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com> wrote:
>
>> That being said, I'll see if I can reduce the memory requirement of the fix.
>
> Done. Managed to reduce the memory requirement of the original code for the lazy case as well. Basically two object references lighter (128 bits).
>
> Thread-safe, lighter and slightly more immutable.
>
>
> -David
>
Re: AnnotatedTypeImpl and thread safety
Posted by David Blevins <da...@gmail.com>.
On May 11, 2013, at 1:59 PM, David Blevins <da...@gmail.com> wrote:
> That being said, I'll see if I can reduce the memory requirement of the fix.
Done. Managed to reduce the memory requirement of the original code for the lazy case as well. Basically two object references lighter (128 bits).
Thread-safe, lighter and slightly more immutable.
-David