Commons OGNL
  1. Commons OGNL
  2. OGNL-224

Performance - Locking and performance problem with OgnlRuntime.findParameterTypes

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 3.0
    • Component/s: Core Runtime
    • Labels:
    • Flags:
      Patch

      Description

      I am using struts2 and under heavy load (around 100 threads) many threads are in BLOCKED state because of OgnlRuntime.findParameterTypes(). The actions we use have a generic superclass like:
      public class PersonalCaptureAction extends DataCaptureAction<PersonalDTO>
      OGNL handles this very bad, it enters
      synchronized (_genericMethodParameterTypesCache)
      all the time, at every property access of the Action. A possible workaround is to introduce another layer of superclass that is not generic.

      I know that in current OGNL trunk (4.0-SNAPSHOT) caching has been rewritten, but Struts2 is using 3.0.5, and maybe it could be fixed as 3.0.6 in the git tree I found:
      https://github.com/jkuhnert/ognl

      I will attach a patch and a testcase.

      1. OgnlRuntimeTest.java
        4 kB
        Pelladi Gabor
      2. OGNL-224.patch
        6 kB
        Pelladi Gabor

        Activity

        Hide
        Pelladi Gabor added a comment -

        Patch to fix this issue

        Show
        Pelladi Gabor added a comment - Patch to fix this issue
        Hide
        Pelladi Gabor added a comment -

        Test that measures performance.
        Results in sec before patch: 13.499, 21.348, 1.966, 3.434, 17.479, 1.161
        Results in sec after patch: 7.375, 0.387, 0.313, 3.409, 1.064, 0.257

        The typical workload in our case was test 5, which went from 17 sec to 1 sec.

        Show
        Pelladi Gabor added a comment - Test that measures performance. Results in sec before patch: 13.499, 21.348, 1.966, 3.434, 17.479, 1.161 Results in sec after patch: 7.375, 0.387, 0.313, 3.409, 1.064, 0.257 The typical workload in our case was test 5, which went from 17 sec to 1 sec.
        Hide
        Simone Tripodi added a comment -

        Sorry for the question: is the patch designed to be applied in current ASF trunk or for the GitHub version?

        Show
        Simone Tripodi added a comment - Sorry for the question: is the patch designed to be applied in current ASF trunk or for the GitHub version?
        Hide
        Pelladi Gabor added a comment -

        It is designed for the github version, in ASF (4.0-SNAPSHOT) the whole caching has been rewritten.

        Show
        Pelladi Gabor added a comment - It is designed for the github version, in ASF (4.0-SNAPSHOT) the whole caching has been rewritten.
        Hide
        Simone Tripodi added a comment -

        thanks for clarifying - I would suggest you so to attach that patch to the GitHub project issue tracker, otherwise it would be not easy to get a chance they notice and apply it.

        Yet another help you could give us, is pinging OGNL committers (some of them are Struts committers) on working towards a 4.0 release!

        Show
        Simone Tripodi added a comment - thanks for clarifying - I would suggest you so to attach that patch to the GitHub project issue tracker, otherwise it would be not easy to get a chance they notice and apply it. Yet another help you could give us, is pinging OGNL committers (some of them are Struts committers) on working towards a 4.0 release!
        Hide
        Lukasz Lenart added a comment -

        I will merge it to OGNL git source and prepare a new version.

        Show
        Lukasz Lenart added a comment - I will merge it to OGNL git source and prepare a new version.
        Hide
        Lukasz Lenart added a comment -

        Or could you prepare a pull request ?

        Show
        Lukasz Lenart added a comment - Or could you prepare a pull request ?
        Hide
        Lukasz Lenart added a comment -

        Done, I was able to merge the patch into current source code

        Show
        Lukasz Lenart added a comment - Done, I was able to merge the patch into current source code
        Hide
        Lukasz Lenart added a comment -

        it'll be included with 3.0.6 release

        Show
        Lukasz Lenart added a comment - it'll be included with 3.0.6 release
        Hide
        Pelladi Gabor added a comment -

        Thank you. Monday and Tuesday were a national holiday in Hungary, that is why I did not read your answer until today.
        By the way, is there any plan to release OGNL 4.0, and Struts2 depending on OGNL 4.0? I have seen some activity in 2011, but this year not so much.

        Show
        Pelladi Gabor added a comment - Thank you. Monday and Tuesday were a national holiday in Hungary, that is why I did not read your answer until today. By the way, is there any plan to release OGNL 4.0, and Struts2 depending on OGNL 4.0? I have seen some activity in 2011, but this year not so much.
        Hide
        Lukasz Lenart added a comment -

        The plan is I started working on that few days ago, basically what's needed is to satisfy RM needs which means all the reports have to be green

        http://commons.apache.org/ognl/project-reports.html

        Show
        Lukasz Lenart added a comment - The plan is I started working on that few days ago, basically what's needed is to satisfy RM needs which means all the reports have to be green http://commons.apache.org/ognl/project-reports.html

          People

          • Assignee:
            Lukasz Lenart
            Reporter:
            Pelladi Gabor
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development