[prev in list] [next in list] [prev in thread] [next in thread]
List: groovy-dev
Subject: Re: [groovy-dev] Drop in performance [was dangerous commit]
From: tog <guillaume.alleon () gmail ! com>
Date: 2007-08-30 22:41:53
Message-ID: a0a50dbb0708301541i7a3b55c0vdc943276a832bc6d () mail ! gmail ! com
[Download RAW message or body]
Jochen,
Performance is now similar to the one out of 1.1-beta-2.
Thanks
Tog
On 8/30/07, Jochen Theodorou <blackdrag@gmx.org> wrote:
>
> Marc Palmer schrieb:
> >
> > On 30 Aug 2007, at 01:23, Alex Tkachman wrote:
> >
> >> Jochen!
> >>
> >> It is obviously your call. If you want I may help you to review.
> >>
> >
> > I seem to remember this sync went in for a specific reason, i.e. a race
> > condition or similar in somebody's application.
>
> yes, because of a multithreading issue when multiple threads are
> requesting the same MetaClass.
>
> The method in question is:
>
> > public MetaClass getMetaClass(Class theClass) {
> > MetaClass answer=null;
> > if (constantMetaClassCount!=0) answer = (MetaClass)
> constantMetaClasses.get(theClass);
> > if (answer!=null) return answer;
> >
> > synchronized (theClass) {
> > answer = (MetaClass) weakMetaClasses.get(theClass);
> > if (answer!=null) return answer;
> > answer = getMetaClassFor(theClass);
> > answer.initialize();
> > weakMetaClasses.put(theClass, answer);
> > return answer;
> > }
> > }
>
> and it was:
>
> > public MetaClass getMetaClass(Class theClass) {
> > MetaClass answer=null;
> > if (constantMetaClassCount!=0) answer = (MetaClass)
> constantMetaClasses.get(theClass);
> > if (answer==null) answer = (MetaClass) weakMetaClasses.get
> (theClass);
> > if (answer!=null) return answer;
> >
> > synchronized (theClass) {
> > answer = getMetaClassFor(theClass);
> > answer.initialize();
> > weakMetaClasses.put(theClass, answer);
> > return answer;
> > }
> > }
>
> the idea was to prevent the synchronized block if a class is already in
> the map. But if multiple threads are requesting the same MetaClass at
> the same time, then each of these threads might go through to the
> synchronized block, resulting in one MetaClass creation for each thread.
>
> So I added a check inside the synchronized block, which was slower for a
> single thread. I am not able to really test multiple threads, because I
> have no machine I can make the tests on and that I would trust to really
> show multi threaded performance. Anyway, I decided to move the check
> completely in the synchronized block then.
>
> But looks like that was no good idea too.
>
> I wanted to not to make the method synchronized, because creating the
> MetaClass takes some time and in this time all threads would be
> blocked... and it is not like with ClassLoader#loadClass, because the
> classes there are requested only one time per class... Here it might be
> one time per call in worst case... If doing many calls on java based
> classes it might happen...
>
> so maybe I should go back to the double checking... then it is slower
> one time, but if the class is available it should be faster.
>
>
> Ok, so I changed this and do a double check now:
>
> > public MetaClass getMetaClass(Class theClass) {
> > MetaClass answer=null;
> > if (constantMetaClassCount!=0) answer = (MetaClass)
> constantMetaClasses.get(theClass);
> > if (answer!=null) return answer;
> > answer = (MetaClass) weakMetaClasses.get(theClass);
> > if (answer!=null) return answer;
> >
> > synchronized (theClass) {
> > answer = (MetaClass) weakMetaClasses.get(theClass);
> > if (answer!=null) return answer;
> > answer = getMetaClassFor(theClass);
> > answer.initialize();
> > weakMetaClasses.put(theClass, answer);
> > return answer;
> > }
> > }
>
> I hope this increase performance again, the method itself is now three
> times faster, if the same class is continuously requested in a single
> thread.
>
> bye blackdrag
>
> --
> Jochen "blackdrag" Theodorou
> Groovy Tech Lead (http://groovy.codehaus.org)
> http://blackdragsview.blogspot.com/
>
> ---------------------------------------------------------------------
> To unsubscribe from this list please visit:
>
> http://xircles.codehaus.org/manage_email
>
>
--
Best Regards
Guillaume
http://cheztog.blogspot.com
[Attachment #3 (text/html)]
Jochen,<br><br>Performance is now similar to the one out of \
1.1-beta-2.<br><br>Thanks<br>Tog<br><br><div><span class="gmail_quote">On 8/30/07, <b \
class="gmail_sendername">Jochen Theodorou</b> <<a href="mailto:blackdrag@gmx.org"> \
blackdrag@gmx.org</a>> wrote:</span><blockquote class="gmail_quote" \
style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; \
padding-left: 1ex;">Marc Palmer schrieb:<br>><br>> On 30 Aug 2007, at 01:23, \
Alex Tkachman wrote: <br>><br>>> Jochen!<br>>><br>>> It is \
obviously your call. If you want I may help you to \
review.<br>>><br>><br>> I seem to remember this sync went in for a \
specific reason, i.e. a race<br> > condition or similar in somebody's \
application.<br><br>yes, because of a multithreading issue when multiple threads \
are<br>requesting the same MetaClass.<br><br>The method in question \
is:<br><br>> public MetaClass getMetaClass(Class theClass) \
{ <br>> MetaClass \
answer=null;<br>> if \
(constantMetaClassCount!=0) answer = (MetaClass) \
constantMetaClasses.get(theClass);<br>> \
if (answer!=null) return \
answer;<br>><br>> synchronized \
(theClass) { <br>> \
answer = (MetaClass) \
weakMetaClasses.get(theClass);<br>> \
if (answer!=null) return \
answer;<br>> \
answer = getMetaClassFor(theClass);<br>> \
answer.initialize ();<br>> \
weakMetaClasses.put(theClass, \
answer);<br>> \
return answer;<br>> \
}<br>> }<br><br>and it \
was:<br><br>> public MetaClass getMetaClass(Class \
theClass) {<br>> MetaClass \
answer=null; <br>> if \
(constantMetaClassCount!=0) answer = (MetaClass) \
constantMetaClasses.get(theClass);<br>> \
if (answer==null) answer = (MetaClass) \
weakMetaClasses.get(theClass);<br>> \
if (answer!=null) return answer; \
<br>><br>> synchronized \
(theClass) {<br>> \
answer = getMetaClassFor(theClass);<br>> \
answer.initialize();<br>> \
weakMetaClasses.put(theClass, \
answer);<br>> \
return answer; <br>> \
}<br>> }<br><br>the idea was to prevent the synchronized \
block if a class is already in<br>the map. But if multiple threads are requesting the \
same MetaClass at<br>the same time, then each of these threads might go through to \
the <br>synchronized block, resulting in one MetaClass creation for each \
thread.<br><br>So I added a check inside the synchronized block, which was slower for \
a<br>single thread. I am not able to really test multiple threads, because I <br>have \
no machine I can make the tests on and that I would trust to really<br>show multi \
threaded performance. Anyway, I decided to move the check<br>completely in the \
synchronized block then.<br><br>But looks like that was no good idea too. <br><br>I \
wanted to not to make the method synchronized, because creating the<br>MetaClass \
takes some time and in this time all threads would be<br>blocked... and it is not \
like with ClassLoader#loadClass, because the<br> classes there are requested only one \
time per class... Here it might be<br>one time per call in worst case... If doing \
many calls on java based<br>classes it might happen...<br><br>so maybe I should go \
back to the double checking... then it is slower <br>one time, but if the class is \
available it should be faster.<br><br><br>Ok, so I changed this and do a double check \
now:<br><br>> public MetaClass getMetaClass(Class \
theClass) {<br>> MetaClass \
answer=null; <br>> if \
(constantMetaClassCount!=0) answer = (MetaClass) \
constantMetaClasses.get(theClass);<br>> \
if (answer!=null) return \
answer;<br>> answer = (MetaClass) \
weakMetaClasses.get(theClass);<br> \
> if (answer!=null) return \
answer;<br>><br>> synchronized \
(theClass) {<br>> \
answer = (MetaClass) \
weakMetaClasses.get(theClass);<br>> \
if (answer!=null) return answer;<br> \
> answer = \
getMetaClassFor(theClass);<br>> \
answer.initialize();<br>> \
weakMetaClasses.put(theClass, \
answer);<br>> \
return answer;<br>> \
}<br>> } <br><br>I hope this increase performance again, \
the method itself is now three<br>times faster, if the same class is continuously \
requested in a single<br>thread.<br><br>bye blackdrag<br><br>--<br>Jochen \
"blackdrag" Theodorou <br>Groovy Tech Lead (<a \
href="http://groovy.codehaus.org">http://groovy.codehaus.org</a>)<br><a \
href="http://blackdragsview.blogspot.com/">http://blackdragsview.blogspot.com/</a><br><br>---------------------------------------------------------------------
<br>To unsubscribe from this list please visit:<br><br> <a \
href="http://xircles.codehaus.org/manage_email">http://xircles.codehaus.org/manage_email</a><br><br></blockquote></div><br><br \
clear="all"><br>-- <br><br>Best Regards <br>Guillaume<br><a \
href="http://cheztog.blogspot.com">http://cheztog.blogspot.com</a>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic