New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update XStream drive to improve performance in xml serilization/deserialization #2561

Merged
merged 2 commits into from Oct 10, 2016

Conversation

3 participants
@Jimilian
Contributor

Jimilian commented Sep 22, 2016

According XStream FAQ (http://x-stream.github.io/faq.html#Scalability):

XStream is a generalizing library, it inspects and handles your types on
the fly. Therefore it will normally be slower than a piece of optimised
Java code generated out of a schema. However, it is possible to increase
the performance anyway:

  • Write custom converters for those of your types that occur very often in
    your XML.
  • Keep a configured XStream instance for multiple usage. Creation and
    initialization is quite expensive compared to the overhead of Stream
    when calling marshall or unmarshal.
  • Use Xpp3 or StAX parsers.

So, I decided to move from old Xpp to new Xpp3.

Update XStream drive to improve performance in xml serilization/deser…
…ialization

According XStream FAQ (http://x-stream.github.io/faq.html#Scalability):

XStream is a generalizing library, it inspects and handles your types on
the fly. Therefore it will normally be slower than a piece of optimized
Java code generated out of a schema. However, it is possible to increase
the performance anyway:

* Write custom converters for those of your types that occur very often in
your XML.
* Keep a configured XStream instance for multiple usage. Creation and
initialization is quite expensive compared to the overhead of XStream
when calling marshall or unmarshal.
* Use Xpp3 or StAX parsers.

So, I decided to move from old Xpp to new Xpp3.
@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Sep 22, 2016

Member

Needs some testing. We're using a slightly outdated XStream fork (https://github.com/jenkinsci/xstream) with some custom patches, hence no guarantee this driver operates similarly to the upstream documentation

Member

oleg-nenashev commented Sep 22, 2016

Needs some testing. We're using a slightly outdated XStream fork (https://github.com/jenkinsci/xstream) with some custom patches, hence no guarantee this driver operates similarly to the upstream documentation

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Sep 22, 2016

Member

What about View#updateByXml and XStreamDOM#writeTo/XStreamDOM#from?

Member

daniel-beck commented Sep 22, 2016

What about View#updateByXml and XStreamDOM#writeTo/XStreamDOM#from?

@Jimilian

This comment has been minimized.

Show comment
Hide comment
@Jimilian

Jimilian Sep 22, 2016

Contributor

@daniel-beck good catch. I didn't search for XppDriver, I just saw that XmlFile needs some CPU time (and memory) in case of big amount of unit tests during profiling. But yes, I think it's better to update all usages. Probably they are equal from stability/quality perspective.

Contributor

Jimilian commented Sep 22, 2016

@daniel-beck good catch. I didn't search for XppDriver, I just saw that XmlFile needs some CPU time (and memory) in case of big amount of unit tests during profiling. But yes, I think it's better to update all usages. Probably they are equal from stability/quality perspective.

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Sep 29, 2016

Member

@Jimilian Marked as work-in-progress according to the last comment

Member

oleg-nenashev commented Sep 29, 2016

@Jimilian Marked as work-in-progress according to the last comment

@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Sep 29, 2016

Member

@oleg-nenashev That was addressed in the second commit.

Member

daniel-beck commented Sep 29, 2016

@oleg-nenashev That was addressed in the second commit.

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Sep 29, 2016

Member

OK, the comment was after the commit and it confused me

Member

oleg-nenashev commented Sep 29, 2016

OK, the comment was after the commit and it confused me

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Sep 29, 2016

Member

👍 , but needs some extensive testing due to our ancient XStream version

Member

oleg-nenashev commented Sep 29, 2016

👍 , but needs some extensive testing due to our ancient XStream version

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Oct 5, 2016

Member

@jenkinsci/code-reviewers @daniel-beck Any additional feedback?
Marking it as ready-for-merge

Member

oleg-nenashev commented Oct 5, 2016

@jenkinsci/code-reviewers @daniel-beck Any additional feedback?
Marking it as ready-for-merge

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Oct 5, 2016

Member

I doubt the fix is a subject for backporting btw

Member

oleg-nenashev commented Oct 5, 2016

I doubt the fix is a subject for backporting btw

@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Oct 5, 2016

Member

I doubt the fix is a subject for backporting btw

Most definitely not.

Member

daniel-beck commented Oct 5, 2016

I doubt the fix is a subject for backporting btw

Most definitely not.

@oleg-nenashev oleg-nenashev merged commit 8888296 into jenkinsci:master Oct 10, 2016

2 of 3 checks passed

Jenkins job PR-2561 This pull request is scheduled to be built
Details
Jenkins This pull request looks good
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@Jimilian Jimilian deleted the Jimilian:update_xstream_driver branch Oct 10, 2016

oleg-nenashev added a commit that referenced this pull request Oct 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment