castor

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. Text File
    patch-C2630-20090127.txt
    26/Jan/09 5:44 PM
    1 kB
    Ralf Joachim
  2. Text File
    patch-CASTOR-2630.txt
    28/Jan/09 4:16 PM
    4 kB
    Michael McMaster
  3. Text File
    patch-CASTOR-2630-20090127-2.txt
    27/Jan/09 2:26 PM
    3 kB
    Joachim Grüneis
  4. Text File
    patch-CASTOR-2630-20090130.txt
    29/Jan/09 6:04 PM
    17 kB
    Joachim Grüneis

Issue Links

is depended upon by

Bug - A problem which impairs or prevents the functions of the product. CASTOR-2614 XML_NAMING configuration cannot be set programmatically

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: