Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.7.x, 2.x
    • Fix Version/s: None
    • Component/s: Engine
    • Labels:

      Description

      When MapGetExecutor is instantiated by the UberspectImpl it iterates through all the interfaces implemented by the object's Class to figure out whether it's a java.util.Map

      This could be replaced by an instance of test on the object received by the Uberspect which would be much faster. We noticed this as a hotspot after upgrading to 1.6 through our Performance Tests JProfiler output.

      I am happy to provide a patch for this, it would be nice to get this on the maintenance branch as well if possible, so that we don't have to maintain a fork until we upgrade.

      1. VELOCITY-815.patch
        6 kB
        Oswaldo Hernandez

        Activity

        Oswaldo Hernandez created issue -
        Hide
        Nathan Bubna added a comment -

        Patches are great. Can you attach screenshots of the before/after performance output?

        Show
        Nathan Bubna added a comment - Patches are great. Can you attach screenshots of the before/after performance output?
        Hide
        Oswaldo Hernandez added a comment -

        Sure Nathan, I am still refining the fix and running it through JProfiler. I will update this issue with the patch and before/after output as soon as I can. Thanks

        Show
        Oswaldo Hernandez added a comment - Sure Nathan, I am still refining the fix and running it through JProfiler. I will update this issue with the patch and before/after output as soon as I can. Thanks
        Oswaldo Hernandez made changes -
        Field Original Value New Value
        Attachment VELOCITY-815.patch [ 12523066 ]
        Hide
        Oswaldo Hernandez added a comment -

        Hi nathan, sorry it's taken me a while to provide this but I have been fairly busy. The patch contains the changes to make MapGetExecutor significantly faster. This used to be an important hotspot in our JProfiler tests and now it takes so little that it does not even appear, can provide a couple of screenshots if you like.

        Let me know if you need any clarification on the changes. They basically consist in using the instance of operator to figure out whether the object is a map or not. For that we made the UberspectImpl pass in the actual Object.

        We also cache the method object obtained through getMethod since this is expensive and should be safe to do as it comes from the system classloader. getMethod has been made lazy so that we only lookup the method object if it's actually needed. According to my reading of the source this is only used for debug logging when bad stuff happens so it should be a speed win for most users.

        Cheers,
        Oswaldo

        Show
        Oswaldo Hernandez added a comment - Hi nathan, sorry it's taken me a while to provide this but I have been fairly busy. The patch contains the changes to make MapGetExecutor significantly faster. This used to be an important hotspot in our JProfiler tests and now it takes so little that it does not even appear, can provide a couple of screenshots if you like. Let me know if you need any clarification on the changes. They basically consist in using the instance of operator to figure out whether the object is a map or not. For that we made the UberspectImpl pass in the actual Object . We also cache the method object obtained through getMethod since this is expensive and should be safe to do as it comes from the system classloader. getMethod has been made lazy so that we only lookup the method object if it's actually needed. According to my reading of the source this is only used for debug logging when bad stuff happens so it should be a speed win for most users. Cheers, Oswaldo
        Jarkko Viinamäki made changes -
        Summary MapGetExecutor is very slow MapGetExecutor is very slow [PATCH]

          People

          • Assignee:
            Unassigned
            Reporter:
            Oswaldo Hernandez
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development