castor
  1. castor
  2. CASTOR-2630

Marshaller performance - loadDefaultProperties

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.3
    • Component/s: XML
    • Labels:
      None
    • Environment:
      32-bit WinXP Pro SP3 using Sun JDK 1.5.0_14
    • Number of attachments :
      4

      Description

      Submitted per request: http://www.nabble.com/Marshaller-performance---loadDefaultProperties-td21633179.html

      I'm looking at some performance issues with our product that's using Castor 1.2. In profiling the code, I'm seeing a majority of the CPU time being spent constructing Marshaller instances. Following the call path, it appears that each new Marshaller is loading and parsing the castor.properties file:

      java.net.URL.openStream 12,450 ms (7 %) 1,138 µs 10,940
      7.6% - 12,450 ms - 10,940 hot spot inv. org.castor.core.util.Configuration.loadFromClassPath (line: 136)
      4.2% - 6,931 ms - 5,474 hot spot inv. org.castor.core.util.Configuration.loadDefaultProperties (line: 49)
      4.2% - 6,931 ms - 5,474 hot spot inv. org.castor.core.CoreConfiguration.<init> (line: 58)
      4.2% - 6,931 ms - 5,474 hot spot inv. org.castor.xml.XMLConfiguration.newInstance (line: 121)
      4.2% - 6,931 ms - 5,474 hot spot inv. org.castor.xml.AbstractInternalContext.<init> (line: 45)
      2.9% - 4,769 ms - 3,650 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 185)
      1.6% - 2,650 ms - 1,830 hot spot inv. org.exolab.castor.xml.MarshalFramework.<init> (line: 298)
      1.3% - 2,119 ms - 1,820 hot spot inv. org.exolab.castor.xml.MarshalFramework.<init> (line: 310)
      1.3% - 2,162 ms - 1,824 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 381)
      3.4% - 5,519 ms - 5,466 hot spot inv. org.castor.core.util.Configuration.loadDefaultProperties (line: 96)
      3.4% - 5,519 ms - 5,466 hot spot inv. org.castor.xml.XMLConfiguration.<init> (line: 59)
      3.4% - 5,519 ms - 5,466 hot spot inv. org.castor.xml.XMLConfiguration.newInstance (line: 121)
      3.4% - 5,519 ms - 5,466 hot spot inv. org.castor.xml.AbstractInternalContext.<init> (line: 45)
      2.1% - 3,447 ms - 3,638 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 185)
      1.3% - 2,072 ms - 1,828 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 381)

      java.util.Properties.load 11,802 ms (7 %) 1,079 µs 10,931
      7.2% - 11,802 ms - 10,931 hot spot inv. org.castor.core.util.Configuration.loadFromClassPath (line: 136)
      4.0% - 6,477 ms - 5,460 hot spot inv. org.castor.core.util.Configuration.loadDefaultProperties (line: 96)
      4.0% - 6,477 ms - 5,460 hot spot inv. org.castor.xml.XMLConfiguration.<init> (line: 59)
      4.0% - 6,477 ms - 5,460 hot spot inv. org.castor.xml.XMLConfiguration.newInstance (line: 121)
      4.0% - 6,477 ms - 5,460 hot spot inv. org.castor.xml.AbstractInternalContext.<init> (line: 45)
      2.5% - 4,115 ms - 3,633 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 185)
      1.4% - 2,238 ms - 1,823 hot spot inv. org.exolab.castor.xml.MarshalFramework.<init> (line: 298)
      1.2% - 1,877 ms - 1,810 hot spot inv. org.exolab.castor.xml.MarshalFramework.<init> (line: 310)
      1.4% - 2,361 ms - 1,827 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 381)
      1.4% - 2,361 ms - 1,827 hot spot inv. org.exolab.castor.xml.Marshaller.initialize (line: 305)
      1.4% - 2,361 ms - 1,827 hot spot inv. org.exolab.castor.xml.Marshaller.<init> (line: 315)
      3.3% - 5,324 ms - 5,471 hot spot inv. org.castor.core.util.Configuration.loadDefaultProperties (line: 49)
      3.3% - 5,324 ms - 5,471 hot spot inv. org.castor.core.CoreConfiguration.<init> (line: 58)
      3.3% - 5,324 ms - 5,471 hot spot inv. org.castor.xml.XMLConfiguration.newInstance (line: 121)
      3.3% - 5,324 ms - 5,471 hot spot inv. org.castor.xml.AbstractInternalContext.<init> (line: 45)
      2.1% - 3,448 ms - 3,643 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 185)
      1.1% - 1,876 ms - 1,828 hot spot inv. org.castor.xml.BackwardCompatibilityContext.<init> (line: 381)

      Is there some reason why the properties files are not read once and cached? My application is spending slightly over 50 percent of its time loading and parsing these files, but I don't see any way to create a Marshaller instance without going through the BackwardCompatibilityContext initialization.

      -Michael


      Hi Michael,

      Could you please open a new issue in jira about this.

      Having said that I don't see a solution to fix this if you use constructors of Marshaller yourself. The prefered way to construct a marshaller instance is through XMLContext.createMarshaller(). But using XMLContext wouldn't help you either at the moment as there seams to be a bug that always reloads properies even if it gets reassigned to the ones loaded at construction of XMLContext directly thereafter.

      Regards
      Ralf

      1. patch-C2630-20090127.txt
        1 kB
        Ralf Joachim
      2. patch-CASTOR-2630.txt
        4 kB
        Michael McMaster
      3. patch-CASTOR-2630-20090127-2.txt
        3 kB
        Joachim Grüneis
      4. patch-CASTOR-2630-20090130.txt
        17 kB
        Joachim Grüneis

        Issue Links

          Activity

          Hide
          Ralf Joachim added a comment -

          Idea to prevent loading of properties at every construction of Marshaller through XMLContext. Having said that I haven't tested anything.

          Show
          Ralf Joachim added a comment - Idea to prevent loading of properties at every construction of Marshaller through XMLContext. Having said that I haven't tested anything.
          Hide
          Werner Guttmann added a comment -

          Ralf, there'd be other options, as far as I can tell, but it would take us some time to change things to use some other pattern(s). In other words, fine by me.

          Show
          Werner Guttmann added a comment - Ralf, there'd be other options, as far as I can tell, but it would take us some time to change things to use some other pattern(s). In other words, fine by me.
          Hide
          Joachim Grüneis added a comment -

          Hello, I have created a similar patch... and I think the solution is totally fine with the modifications I did earlier

          But I didn't sent it in as I'm currently blocked at testing.
          This test fails:
          1) mapping/constructor/primitive/Mapping-Constructors#Test02_ReferenceDocument(org.castor.xmlctf.TestWithReferenceDocume
          nt)java.lang.IllegalArgumentException: File '.\target\xmlctf\MasterTestSuite\mapping\constructor\primitive/input.without
          .xml' does not exist

          simply renaming "input.without.xml" into "input-without.xml" doesn't help as the results differ

          the rest of the xmlctf test suit runs fine with my - similar - patch

          can someone help me with the failing test?

          afterwards I'll checkin the patch

          Show
          Joachim Grüneis added a comment - Hello, I have created a similar patch... and I think the solution is totally fine with the modifications I did earlier But I didn't sent it in as I'm currently blocked at testing. This test fails: 1) mapping/constructor/primitive/Mapping-Constructors#Test02_ReferenceDocument(org.castor.xmlctf.TestWithReferenceDocume nt)java.lang.IllegalArgumentException: File '.\target\xmlctf\MasterTestSuite\mapping\constructor\primitive/input.without .xml' does not exist simply renaming "input.without.xml" into "input-without.xml" doesn't help as the results differ the rest of the xmlctf test suit runs fine with my - similar - patch can someone help me with the failing test? afterwards I'll checkin the patch
          Hide
          Werner Guttmann added a comment -

          Odd, This is a new test I recently checked in as part of a bug fix, and that test runs fine with me. Are you having the problems after applying the patch, or with a vanilla checkout ?

          Show
          Werner Guttmann added a comment - Odd, This is a new test I recently checked in as part of a bug fix, and that test runs fine with me. Are you having the problems after applying the patch, or with a vanilla checkout ?
          Hide
          Werner Guttmann added a comment -

          And what does your patch look like ?

          Show
          Werner Guttmann added a comment - And what does your patch look like ?
          Hide
          Werner Guttmann added a comment -

          Just committed a patch that fixes the problem with the test case. In other words, the XMLCTF now runs fine again. I have to admit, though, that I have got several projects checked out where it runs as well.

          Show
          Werner Guttmann added a comment - Just committed a patch that fixes the problem with the test case. In other words, the XMLCTF now runs fine again. I have to admit, though, that I have got several projects checked out where it runs as well.
          Hide
          Joachim Grüneis added a comment -

          As I said... the same as the patch of Ralf... just slightly different...

          Show
          Joachim Grüneis added a comment - As I said... the same as the patch of Ralf... just slightly different...
          Hide
          Joachim Grüneis added a comment -

          I had a really vanilla checkout, even no maven repository - and the test failed.
          But now things are fine

          Show
          Joachim Grüneis added a comment - I had a really vanilla checkout, even no maven repository - and the test failed. But now things are fine
          Hide
          Joachim Grüneis added a comment -

          The two patches solve the problem only on the surface - a second closer look and test test showed me that still multiple instances of BackwardCompatibility... are created at "new Marshaller()". This is due the fact that many Castor objects rely to be fully configured after the constructor and there is no clear path how to hand down information...
          I continue to work on this issue but now things get harder to solve, so please be patient.

          Show
          Joachim Grüneis added a comment - The two patches solve the problem only on the surface - a second closer look and test test showed me that still multiple instances of BackwardCompatibility... are created at "new Marshaller()". This is due the fact that many Castor objects rely to be fully configured after the constructor and there is no clear path how to hand down information... I continue to work on this issue but now things get harder to solve, so please be patient.
          Hide
          Werner Guttmann added a comment -

          No problem. I spotted yet another location an UnmarshalHandler yesterday. In other words, once we have a feasable approach, there's more than one place to apply to.

          Show
          Werner Guttmann added a comment - No problem. I spotted yet another location an UnmarshalHandler yesterday. In other words, once we have a feasable approach, there's more than one place to apply to.
          Hide
          Michael McMaster added a comment -

          My application is locked to Castor 1.2, so I've taken a quick pass at eliminating the bottleneck with reloading/reparsing the properties files. I'm interested in getting your opinion on this change, which essentially adds a Properties cache to the Configuration class. My performance testing with this change yielded a pretty dramatic improvement in CPU utilization.

          Thanks
          -Michael

          Show
          Michael McMaster added a comment - My application is locked to Castor 1.2, so I've taken a quick pass at eliminating the bottleneck with reloading/reparsing the properties files. I'm interested in getting your opinion on this change, which essentially adds a Properties cache to the Configuration class. My performance testing with this change yielded a pretty dramatic improvement in CPU utilization. Thanks -Michael
          Hide
          Joachim Grüneis added a comment -

          I'm still working on a patch for 1.3.
          This version is improved as it does no longer instantiate BackwardC...InternalContext if XMLContext is used.
          The naming setting is also correctly used.

          Show
          Joachim Grüneis added a comment - I'm still working on a patch for 1.3. This version is improved as it does no longer instantiate BackwardC...InternalContext if XMLContext is used. The naming setting is also correctly used.
          Hide
          Joachim Grüneis added a comment -

          I think my last message is/was confusing...
          I meant that I continue to work on the patch for 1.3...

          Show
          Joachim Grüneis added a comment - I think my last message is/was confusing... I meant that I continue to work on the patch for 1.3...
          Hide
          Joachim Grüneis added a comment -

          Any feedback according to patch patch-CASTOR-2630-20090130.txt?

          Show
          Joachim Grüneis added a comment - Any feedback according to patch patch- CASTOR-2630 -20090130.txt?
          Hide
          Werner Guttmann added a comment -

          I was holding back with comments, as I interpreted your 'I continue to ..' as an indicator that you have not finished the patch.

          Having said that, I am not sure whether the use of a check against null in a constructor is the right way to go. Why not supplying two constructors at MarshalFramework, and invoke the one with an InternalContext as parameter explicitly ?

          Show
          Werner Guttmann added a comment - I was holding back with comments, as I interpreted your 'I continue to ..' as an indicator that you have not finished the patch. Having said that, I am not sure whether the use of a check against null in a constructor is the right way to go. Why not supplying two constructors at MarshalFramework, and invoke the one with an InternalContext as parameter explicitly ?
          Hide
          Joachim Grüneis added a comment -

          For 1.3 I've committed the patch fixing this behaviour.

          Show
          Joachim Grüneis added a comment - For 1.3 I've committed the patch fixing this behaviour.
          Hide
          Werner Guttmann added a comment -

          Patch has apparently been committed.

          Show
          Werner Guttmann added a comment - Patch has apparently been committed.

            People

            • Assignee:
              Joachim Grüneis
              Reporter:
              Michael McMaster
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: