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

Speedup gradle test output XML processing #508

Merged
merged 8 commits into from Feb 3, 2017

Conversation

Projects
None yet
3 participants
@jonnyzzz
Member

jonnyzzz commented Feb 2, 2017

As discussed in the https://youtrack.jetbrains.com/issue/IDEA-167449 issue I decided to implement faster XML processing for Gradle plugin that would avoid my most pain point for tests with heavy output flow.

Before the fix, IDEA UI was too overloaded. According to Activity Monitor IDEA process was eating even more CPU than tests worker process. You may find snapshot in the issue.

After the fix I see It consumes less CPU wither by IDEA UI responsiveness or by fresh CPU profiling.

Current implementation I use XStreams (it was included in classpath) Hierarchical parser that uses KXML pull parser implementation. The whole processing is done within one class.

Unfortunately, I was not able to implement integration tests to measure performance boost more technically as well as to make sure performance will not be lost again. Also, it looks like there are NO tests for test runner integration at all. Just let me know if you have some correct snippet to implement more test.

jonnyzzz added some commits Feb 1, 2017

Move GradleTestsExecution#onOutput method to a standalone class to si…
…mplify testability

Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
drop unused code, simplify
Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
consolidate all XML processing into XmlXpathHelper
Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
move helper closer to event classes
Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>

@vladsoroka vladsoroka self-assigned this Feb 2, 2017

@vladsoroka

It would be better to introduce a new interface for text event message handler it should be format independent. Do not remove xpath implemetation for futher comparison. Add new implementation based on kxm library.
This will allow us easier switch the implementation (e.g. using binary format or other)

And also we need a performance test to be sure we are improving things.

jonnyzzz added some commits Feb 2, 2017

extract view for XML to implement unit-performance test to compare pe…
…rformance between existing and new XML parsing code

Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
cleanup extracted API
Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
add KXML implementation for parsing + tests
Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
use XPP parser (faster than KXml2 on big messages) for gradle output …
…parsing

Signed-off-by: Eugene Petrenko <eugene.petrenko@gmail.com>
@jonnyzzz

This comment has been minimized.

Show comment
Hide comment
@jonnyzzz

jonnyzzz Feb 2, 2017

Member

I have force updated commits. Issues should be resolved. We no longer need KXml2 parser as XStream XPP works faster on my tests, which I have added

Member

jonnyzzz commented Feb 2, 2017

I have force updated commits. Issues should be resolved. We no longer need KXml2 parser as XStream XPP works faster on my tests, which I have added

@vladsoroka

This comment has been minimized.

Show comment
Hide comment
@vladsoroka

vladsoroka Feb 3, 2017

Contributor

Merged, thanks for the contribution!

Contributor

vladsoroka commented Feb 3, 2017

Merged, thanks for the contribution!

@SergeyZh SergeyZh merged commit d452937 into JetBrains:master Feb 3, 2017

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