Uploaded image for project: 'XalanJ2'
  1. XalanJ2
  2. XALANJ-2253

org.apache.xalan.xsltc.trax.TemplatesImpl.newTransformer() is needlessly synchronized and causes a bottleneck under multithreaded load

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 2.7.1, 2.7
    • None
    • XSLTC
    • None
    • PatchAvailable

    Description

      While doing profiling on an in house application using OptimizeIT, I noticed that the newTransformer method on org.apache.xalan.xsltc.trax.TemplatesImpl is creating a bottleneck because it is synchronized. I fiddled with the code a bit and was able to remove the synchronization which improved performance about 15% for the 20 threads in my test harness (test harness class is attached, however, I have been testing on proprietary style sheets, so can't include those. These style sheets are fairly complex, so I believe that there will be a more profound time difference when using style sheets that transform more quickly because the proportion of time doing the xform vs the blocking on newTransformer's monitor will be smaller).

      While the newTransformer() method does directly access several member variables, these can fairly easily be made final so as not to require locking. In other places, there was only one instance where I could not make a variable (_uriResolver) final because it was set in a manual de-serialization method (readObject). I did in this case manage to remove any mutation of the variable except in the constructor and aforementioned readObject method which occur before the object is accessible to any thread other than the one creating the object. Much of the need for mutable variables came from lazy initialization which I moved into the appropriate constructor. I replaced arrays with unmodifiable lists to ensure that unintended modification my other threads can not happen. I also noted that the TemplatesImpl class is using org.apache.xalan.xsltc.runtime.Hashtable - shouldn't this have been replaced with the more standard java.util.HashMap by now?! I made this replacement in the classes that I had to touch anyway.

      I used the aforementioned harness for testing, but could not run the tests found in the tests folder of module because these are broken as checked out and will not compile. I removed several mutator methods that were not being called from anywhere and prevented making a variable final. Someone expertly familiar with the classes i touched should review them to ensure that there are no insidious issues introduced.

      Here is the information required persuant to section 7.4 of the xalan charter:

      Name: Nathan Pahucki
      Empoloyer: Novell, Inc.
      Are you the author of the code being contributed? : Yes, I modified existing code.
      Do you have the right to grant the copyright and patent licenses for the contribution that are set forth in the ASF v.2.0 license (http://www.apache.org/licenses/LICENSE-2.0)? Yes
      Does your employer have any rights to code that you have written, for example, through your contract for employment? If so, has your employer given you permission to contribute the code on its behalf or waived its rights in the code? ?/Yes - I have gotten permission from our Open Source Review Board and legal team.
      Are you aware of any third-party licenses or other restrictions (such as related patents or trademarks) that could apply to your contribution? If so, what are they? No

      Attachments

        1. TestXSLTC.java
          8 kB
          Nathan Pahucki
        2. XALANJ-2253.patch
          22 kB
          Nathan Pahucki

        Activity

          People

            Unassigned Unassigned
            npahucki Nathan Pahucki
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated: