Significantly improved performance of DocumentUtilities #12#13
Conversation
|
You may check performance just running existing unit tests. Initialisation takes the same time but other tests are 10-20 times faster than before. |
| private static final String XML_SCHEMA_INSTANCE = "http://www.w3.org/2001/XMLSchema-instance"; | ||
| private static final String XML_NAMESPACE = "http://www.w3.org/2000/xmlns/"; | ||
|
|
||
| private static final JAXBContext STIX_CONTEXT = initDefaultContext(); |
There was a problem hiding this comment.
It can be initialized lazily, if needed. @nemonik WDYT?
|
@nemonik can you look at this and merge if you think it's OK? You should have permission to merge it, but if not, let me know. |
|
|
||
| private static JAXBContext initDefaultContext(){ | ||
| try { | ||
| return JAXBContext.newInstance("org.mitre.stix.stix_1"); |
There was a problem hiding this comment.
This JAXBContext instance seems very coupled to just the STIX_1 namespace. I noticed that other generated binding classes use the same DocumentUtilitites.toDocument() method (i.e. types org.mitre.cybox.cybox_2).
Will this JAXBContext still work in the case where the code is just calling the toDocument method on the cybox object? The cybox binding is under the package namespace that starts with "org.mitre" but is not "org.mitre.stix.stix_1".
There was a problem hiding this comment.
It should work. STIXPackage is the most general container that comprises all other types in org.mitre, and it is in org.mitre.stix.stix_1. So when JAXBContext is initialised it should scan all the types in org.mitre.stix.stix_1 and all other types that are referenced by STIXPackage recursively and irrespectively to the initial package. So all types in full org.mitre are in the context. Please let me know if I'm wrong, because I'm not an expert in JAXB. The unit tests which construct STIXPackage with cybox contents and then invoke toDocument in the project would probably fail if I was wrong.
There was a problem hiding this comment.
I do not appear to be as familiar with the JAXB contextPath's class scanning and loading process as you. I do agree with your point on the "org.mitre" and working with the simple vanilla STIX package and cybox indicator element.
I'm not a maintainer and/or familiar with the STIX v1 binding jar as a whole. Inspecting the built binding jar and the generated classes, there are definitely other package namespaces that do not start with "org.mitre" and their classes use/call the DocumentUtilitites.toDocument() utility method. Theoretically, they could possibly fall into the same issue of always instantiating a new JAXBContext.
This comes back to the possible recommendation of DocumentUtilities just maintaining an internal lazy loaded cache of called upon JAXBContext objects for marshaling. This approach would make that one line of code not so coupled/fixed to the context namespace of "org.mitre.stix.stix_1" and starts with "org.mitre" condition. This is probably minor, but just a suggestion to cover other unexpected/possible usage of this binding jar.
There was a problem hiding this comment.
I don't think these elements from other packages will be (often) directly converted with toDocument, I regard it as unlikely case. To me it is much more sensible to use it with STIXPackage. As for me, I think, that some more comments should be added to this method, preventing users from converting any possible element with it.
| private static final String XML_SCHEMA_INSTANCE = "http://www.w3.org/2001/XMLSchema-instance"; | ||
| private static final String XML_NAMESPACE = "http://www.w3.org/2000/xmlns/"; | ||
|
|
||
| public static JAXBContext stixJaxbContext() { |
There was a problem hiding this comment.
I made this method public intentionally, because I lacked it in my project and find it useful for users who want more control over marshalling/unmarshalling.
|
Can you guys please release the latest changes? |
4 participants
Refs #12