groovy
  1. groovy
  2. GROOVY-5276

Change ProxyGenerator to use ASM based class generation instead of GroovyShell

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-beta-3
    • Component/s: None
    • Labels:
    • Number of attachments :
      0

      Description

      The current way of generating proxies, for example when you convert a map to an object using the "as" keyword is highly inefficient. Generating a proxy for a concrete class, in that situation, is two orders of magnitude slower than generating a proxy for an interface with the JDK proxy mechanism.

      This can be improved in two way:

      • use ASM based class generation instead of creating a string which is converted to a class thanks to a GroovyShell
      • implement proxy class caching so that subsequent conversions of similar objects reuse the same generated class instead of always generating a new class

      The pull request (see comment) is such an implementation, which performs way better than the legacy version. For example, using the following script:

      interface Foo {}
      abstract class AbstractFoo {}
      abstract class SingleAbstract { abstract int foo() }
      
      def tests = [
      'mapAsInterface': { [:] as Foo },
      'mapAsAbstractClass': { i -> [:] as AbstractFoo },
      'mapWithOneMethodAsAbstractClass': { i -> ['foo': { i } ] as SingleAbstract }
      ]
      
      tests.each { name, test ->
           long sd = System.currentTimeMillis()
           1000.times { test(it) }
           long dur = System.currentTimeMillis()-sd
           println "$name: ${dur}ms"
      }
      

      Groovy master shows:

      mapAsInterface: 3ms
      mapAsAbstractClass: 951ms
      mapWithOneMethodAsAbstractClass: 968ms
      

      with patch:

      mapAsInterface: 1ms
      mapAsAbstractClass: 8ms
      mapWithOneMethodAsAbstractClass: 8ms
      

      I'm using a pull request and adding a breaking label to this issue because even if all the unit tests pass, I suspect the test coverage is insufficient so I'd be keen for code review.

        Activity

        Hide
        CÚdric Champeau added a comment -
        Show
        CÚdric Champeau added a comment - Pull request: https://github.com/groovy/groovy-core/pull/20

          People

          • Assignee:
            CÚdric Champeau
            Reporter:
            CÚdric Champeau
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: