groovy
  1. groovy
  2. GROOVY-3557

Remove synchronization of isModified method in ExpandoMetaClass to improve performance

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.6, 1.6.1, 1.5.8, 1.6.2, 1.6.3
    • Fix Version/s: 1.9-beta-4, 1.8.4
    • Component/s: None
    • Labels:
      None
    • Environment:
      Linux, Java 1.6, Tomcat
    • Number of attachments :
      1

      Description

      Sscalability seems to suffer as a result of TagLibs being implemented as singletons and locking occurring on ExpandoMetaClass.isModified() when the server is processing multiple requests concurrently.

      Performance is fine when there is a small number of concurrent requests, but as the number of concurrent requests increases response times increase dramatically.

      Locking was noticed by profiling a running application using Your Kit Java Profiler and JMeter to simulate load.

      More details are available here:
      http://digitalmorphosis.blogspot.com/2009/05/grails-scalability.html

      Obvious fix would be to create new instances of taglibs for each request/thread, but not sure what side effects this would have?

        Activity

        Hide
        Serge P. Nekoval added a comment -

        Sorry, this ticket is closed but I don't think it really fixes the problem with locking.

        Just a note, lots of Grails code flows through InvokerHelper which results in the following stack:

        "http-apr-8080"-exec-144
        sun.misc.Unsafe.park(Native Method)
        java.util.concurrent.locks.LockSupport.park(LockSupport.java:158)
        java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:811)
        java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(AbstractQueuedSynchronizer.java:842)
        java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1178)
        org.codehaus.groovy.util.LockableObject.lock(LockableObject.java:34)
        org.codehaus.groovy.reflection.ClassInfo.lock(ClassInfo.java:268)
        org.codehaus.groovy.reflection.ClassInfo.getMetaClass(ClassInfo.java:193)
        org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getMetaClass(MetaClassRegistryImpl.java:214)
        org.codehaus.groovy.runtime.InvokerHelper.getMetaClass(InvokerHelper.java:747)
        org.codehaus.groovy.runtime.InvokerHelper.invokePojoMethod(InvokerHelper.java:780)
        org.codehaus.groovy.runtime.InvokerHelper.invokeMethod(InvokerHelper.java:772)
        org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation.castToBoolean(DefaultTypeTransformation.java:156)
        org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation.booleanUnbox(DefaultTypeTransformation.java:65)
        

        We have a Grails application serving hundreds of requests per second and this seems to be the most critical hot spot. As you can see, locking occurs in ClassInfo.getMetaClass, probably THAT is the right place for using ReadWriterLock.

        Show
        Serge P. Nekoval added a comment - Sorry, this ticket is closed but I don't think it really fixes the problem with locking. Just a note, lots of Grails code flows through InvokerHelper which results in the following stack: "http-apr-8080" -exec-144 sun.misc.Unsafe.park(Native Method) java.util.concurrent.locks.LockSupport.park(LockSupport.java:158) java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:811) java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(AbstractQueuedSynchronizer.java:842) java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1178) org.codehaus.groovy.util.LockableObject.lock(LockableObject.java:34) org.codehaus.groovy.reflection.ClassInfo.lock(ClassInfo.java:268) org.codehaus.groovy.reflection.ClassInfo.getMetaClass(ClassInfo.java:193) org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getMetaClass(MetaClassRegistryImpl.java:214) org.codehaus.groovy.runtime.InvokerHelper.getMetaClass(InvokerHelper.java:747) org.codehaus.groovy.runtime.InvokerHelper.invokePojoMethod(InvokerHelper.java:780) org.codehaus.groovy.runtime.InvokerHelper.invokeMethod(InvokerHelper.java:772) org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation.castToBoolean(DefaultTypeTransformation.java:156) org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation.booleanUnbox(DefaultTypeTransformation.java:65) We have a Grails application serving hundreds of requests per second and this seems to be the most critical hot spot. As you can see, locking occurs in ClassInfo.getMetaClass , probably THAT is the right place for using ReadWriterLock.
        Hide
        blackdrag blackdrag added a comment -

        The issue was about isModified synchronization in EMC, this is done. What you talk about is a new issue, so please make a new issue then.

        Show
        blackdrag blackdrag added a comment - The issue was about isModified synchronization in EMC, this is done. What you talk about is a new issue, so please make a new issue then.
        Hide
        Lari Hotari added a comment -

        Serge, your stacktrace looks like GROOVY-5059 problem. It's fixed in Groovy 1.8.4 (and Grails 2.0).

        Show
        Lari Hotari added a comment - Serge, your stacktrace looks like GROOVY-5059 problem. It's fixed in Groovy 1.8.4 (and Grails 2.0).
        Hide
        Serge P. Nekoval added a comment -

        Lari, yes and no. AFAIK GROOVY-5059 is an improvement for boolean casts where source object is itself Boolean. When source object is not Boolean, it has to extract MetaClass anyway, invoking lock() and then getMetaClassUnderLock(). Also there are many other cases where InvokerHelper is used. For example, I think the following loop is not going to be improved by GROOVY-5059:

        ['1','2',3'].any{!it}

        .

        Since it's a different issue I've created GROOVY-5249 hoping that someone can take a look...

        Show
        Serge P. Nekoval added a comment - Lari, yes and no. AFAIK GROOVY-5059 is an improvement for boolean casts where source object is itself Boolean. When source object is not Boolean, it has to extract MetaClass anyway, invoking lock() and then getMetaClassUnderLock() . Also there are many other cases where InvokerHelper is used. For example, I think the following loop is not going to be improved by GROOVY-5059 : ['1','2',3'].any{!it} . Since it's a different issue I've created GROOVY-5249 hoping that someone can take a look...
        Hide
        Lari Hotari added a comment -

        Serge, agree. I should have said "improved in Groovy 1.8.4" instead of "fixed".

        Show
        Lari Hotari added a comment - Serge, agree. I should have said "improved in Groovy 1.8.4" instead of "fixed".

          People

          • Assignee:
            blackdrag blackdrag
            Reporter:
            Mark Smithson
          • Votes:
            2 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: