groovy
  1. groovy
  2. GROOVY-5428

Performance problem: ClassCastExceptions are created in normal execution flow which kills performance

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.6
    • Fix Version/s: 2.0-beta-3, 1.8.7
    • Component/s: None
    • Labels:
      None
    • Environment:
      Grails 2.0.3 + Groovy 1.8.6
    • Number of attachments :
      3

      Description

      This problem is a major performance bottleneck in Grails 2.0 applications.
      Currently this problem shows up when resources plugin has been installed and used in a Grails app. (see attached screenshot)

      Exceptions are relatively heavy weight in Java. Filling the stacktrace takes most of the time. Normal execution flow should never throw exceptions.
      (it is possible to disable filling the stacktrace like has been done for MissingMethodExceptionNoStack, in that case the performance overhead is usually acceptable.)

      The problem shows up whenever there is 2 classes sharing the same base class and when the other class is a Java class and the other one is a Groovy class (GroovyObject).

      I was able to reproduce the problem with this code. Run this code in a debugger and make a breakpoint for the construction of ClassCastException.
      In this example the base class is java.util.Map , the Java class is LinkedHashMap and the Groovy class is MyMap (extends LinkedHashMap).

      class ClassCastProblem {
          def doCalls() {
              def m1=[abc:'test']
              def m2=new MyMap()
              doSomething(m1)
              doSomething(m2)
              doSomething(m1)
              doSomething(m2)
              doSomething(m1)
          }
          
          void doSomething(m) {
              m.remove('test')
          }
      
          public static void main(String[] args) {
              def ccp=new ClassCastProblem()
              ccp.doCalls()
          }
      }
      
      class MyMap extends LinkedHashMap {
          
      }
      

      In a Grails app the 2 different java.util.Map implementations usually triggering this problem are LinkedHashMap and GrailsParameterMap (GroovyObject).

      In Grails core I've worked around some of the problem with this type of hack:
      https://github.com/grails/grails-core/commit/62626921ebd80e3cdeb9776fd79be7eb92f88763
      At that time I didn't have time to investigate the reason that caused the problem and just made some changes until the problem went away.

        Activity

        Hide
        blackdrag blackdrag added a comment -

        The performance improvement will be much less if you replace the cast with an instanceof and cast, followed by getting the metaclass, instead of calling the intrinsic getClass() method.

        Even if the methods are cached, there is still the issue with having to go through method selecton every time. I was asking what you compare with, because the reference should be the case with cached method calls (ie only m2 and no m1). And there I expect like half the time needed, if not less.

        Show
        blackdrag blackdrag added a comment - The performance improvement will be much less if you replace the cast with an instanceof and cast, followed by getting the metaclass, instead of calling the intrinsic getClass() method. Even if the methods are cached, there is still the issue with having to go through method selecton every time. I was asking what you compare with, because the reference should be the case with cached method calls (ie only m2 and no m1). And there I expect like half the time needed, if not less.
        Hide
        blackdrag blackdrag added a comment -

        Lari, I added the instanceof check for now. Should we close the issue with that?

        Show
        blackdrag blackdrag added a comment - Lari, I added the instanceof check for now. Should we close the issue with that?
        Hide
        Lari Hotari added a comment -

        Yes it's ok to close this one.

        btw. I wonder if the SoftReference pogoCallSiteConstructor and pojoCallSiteConstructor fields of CachedMethod could get GCed under memory pressure and that would cause CachedMethod to generate a new callsite class (bytecode) for the same method (if the callsite gets swapped). I've never seen that happen but that possible problem came into mind.

        Show
        Lari Hotari added a comment - Yes it's ok to close this one. btw. I wonder if the SoftReference pogoCallSiteConstructor and pojoCallSiteConstructor fields of CachedMethod could get GCed under memory pressure and that would cause CachedMethod to generate a new callsite class (bytecode) for the same method (if the callsite gets swapped). I've never seen that happen but that possible problem came into mind.
        Hide
        blackdrag blackdrag added a comment -

        can they be GCed? well.... in a test like yours unlikely, because there is no or almost no object creation between the method calls and during the method call it surely will not happen, since the object is "in use". But after a while, or after lots of memory usage this can happen, yes. And yes, the swapped out call site is even more likely to be collected. But without the SoftReference the ChachedClass object would just become bigger and then we would not only have to create the call site again, we would also have to create the cached class again.

        Show
        blackdrag blackdrag added a comment - can they be GCed? well.... in a test like yours unlikely, because there is no or almost no object creation between the method calls and during the method call it surely will not happen, since the object is "in use". But after a while, or after lots of memory usage this can happen, yes. And yes, the swapped out call site is even more likely to be collected. But without the SoftReference the ChachedClass object would just become bigger and then we would not only have to create the call site again, we would also have to create the cached class again.
        Hide
        blackdrag blackdrag added a comment -

        fixed with the instanceof check only

        Show
        blackdrag blackdrag added a comment - fixed with the instanceof check only

          People

          • Assignee:
            blackdrag blackdrag
            Reporter:
            Lari Hotari
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: