Issue Details (XML | Word | Printable)

Key: CASTOR-2630
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Blocker Blocker
Assignee: Joachim Grüneis
Reporter: Michael McMaster
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
castor

Marshaller performance - loadDefaultProperties

Created: 26/Jan/09 09:59 AM   Updated: 10/Feb/09 12:16 PM   Resolved: 07/Feb/09 05:26 PM
Return to search
Component/s: XML
Affects Version/s: 1.2
Fix Version/s: 1.3

Time Tracking:
Not Specified

File Attachments: 1. Text File patch-C2630-20090127.txt (1 kB)
2. Text File patch-CASTOR-2630-20090127-2.txt (3 kB)
3. Text File patch-CASTOR-2630-20090130.txt (17 kB)
4. Text File patch-CASTOR-2630.txt (4 kB)

Environment: 32-bit WinXP Pro SP3 using Sun JDK 1.5.0_14
Issue Links:
dependent
 


 Description  « Hide

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



Ralf Joachim added a comment - 26/Jan/09 05:44 PM

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


Werner Guttmann added a comment - 27/Jan/09 07:21 AM

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.


Joachim Grüneis added a comment - 27/Jan/09 08:45 AM

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


Werner Guttmann added a comment - 27/Jan/09 09:02 AM

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 ?


Werner Guttmann added a comment - 27/Jan/09 09:02 AM

And what does your patch look like ?


Werner Guttmann added a comment - 27/Jan/09 10:08 AM

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.


Joachim Grüneis added a comment - 27/Jan/09 02:26 PM

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


Joachim Grüneis added a comment - 27/Jan/09 02:27 PM

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


Joachim Grüneis added a comment - 28/Jan/09 02:17 AM

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.


Werner Guttmann added a comment - 28/Jan/09 03:45 AM

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.


Michael McMaster added a comment - 28/Jan/09 04:16 PM

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


Joachim Grüneis added a comment - 29/Jan/09 06:04 PM

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.


Joachim Grüneis added a comment - 30/Jan/09 04:41 AM

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


Joachim Grüneis added a comment - 04/Feb/09 04:22 PM

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


Werner Guttmann added a comment - 04/Feb/09 04:47 PM

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 ?


Joachim Grüneis added a comment - 05/Feb/09 04:36 PM

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


Werner Guttmann added a comment - 07/Feb/09 05:26 PM

Patch has apparently been committed.