Announcer too slow
14 views
Skip to first unread message
j.r.
Jan 31, 2016, 1:01:02 PM1/31/16
to concordion-dev
After we have solved the last one successfully, here comes the next design challenge. :-)
Dynamic proxies are very slow on the .NET platform, because they are realized with dynamic code generation that is compiled on run time. This causes a lot of overhead (e.g. the compiler and all necessary components have to be setup even for the execution of a single test).
My first simple approach is to replace the Announcer with just a List. This seems to work from the functionality perspective. It is a lot faster on the .NET platform. Additionally, the code does not change very much. Adding and removing listeners is very similar. The major difference is with announcing events / calling listeners. But this code is always well encapsulated in dedicated methods such as DocumentParser.announceBeforeParsing(), AssertEqualsCommand.announceSuccess(), ThrowableCaughtPublisher.throwableCaught(), etc.
You can have a look on the proposed changes here:
https://github.com/ShaKaRee/concordion/commit/33d4699ff1e44943ad9cba8d0eff0503cb2aa22c#diff-045b4469b168ea6fe78f51c2755d6d61
If you prefer another solution, please feel free to prepare a pull request. :D
Dynamic proxies are very slow on the .NET platform, because they are realized with dynamic code generation that is compiled on run time. This causes a lot of overhead (e.g. the compiler and all necessary components have to be setup even for the execution of a single test).
My first simple approach is to replace the Announcer with just a List. This seems to work from the functionality perspective. It is a lot faster on the .NET platform. Additionally, the code does not change very much. Adding and removing listeners is very similar. The major difference is with announcing events / calling listeners. But this code is always well encapsulated in dedicated methods such as DocumentParser.announceBeforeParsing(), AssertEqualsCommand.announceSuccess(), ThrowableCaughtPublisher.throwableCaught(), etc.
You can have a look on the proposed changes here:
https://github.com/ShaKaRee/concordion/commit/33d4699ff1e44943ad9cba8d0eff0503cb2aa22c#diff-045b4469b168ea6fe78f51c2755d6d61
If you prefer another solution, please feel free to prepare a pull request. :D
Tim Wright
Jan 31, 2016, 1:30:09 PM1/31/16
to j.r., concordion-dev
* This would be so much easier in Java8 with lambdas. Then we could keep the announcer class.
* Best to wrap the list creation in Collections.synchronizedList( ... ). With the parallel runner, we'll need that :)--
You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.
To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/22dd33f1-a7f3-4be0-8a55-ffd5d19b333a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
--
Tim
021 251 5593
Nigel Charman
Feb 1, 2016, 5:06:44 AM2/1/16
to concord...@googlegroups.com
I do like the elegance of Announcer,
but happy to sacrifice it if it makes a quantifiable improvement
to the .NET implementation. Do you have any before and after
performance figures?
@tim - since the parallel runner is per specification, and each specification gets a new set of listeners from ConcordionBuilder, I don't believe the listeners need synchronising at the moment.
However, synchronising the listeners would go part way towards making it safe to parallelise examples (I haven't looked in detail to what changes would be needed to support this).
@tim - since the parallel runner is per specification, and each specification gets a new set of listeners from ConcordionBuilder, I don't believe the listeners need synchronising at the moment.
However, synchronising the listeners would go part way towards making it safe to parallelise examples (I haven't looked in detail to what changes would be needed to support this).
To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/CAJifTyqYFJhxvfu_WF6Mj0nKk%2B9PSqk4QpZB4NO0xtxDAc4_VA%40mail.gmail.com.
j.r.
Feb 1, 2016, 11:35:30 AM2/1/16
to concordion-dev
Hi Tim,
Thank you for your comments.
I like the idea of using Collections.synchronizedList() to make the code robust.
Thank you for your comments.
I like the idea of using Collections.synchronizedList() to make the code robust.
j.r.
Feb 1, 2016, 12:05:33 PM2/1/16
to concordion-dev
Hi Nigel,
Thank you for your support to change the Announcer.
Yes, performance optimizations should be done based on measurements. I have used a profiler to identify spots for performance improvement.
One single Concordion.NET test takes about 600 - 900 ms after the proposed performance optimizations.
The call of Announcer took 1460 ms according to profiler data. The list based solution needs 5 ms.
The classLoader.getResourceAsStream() based implemenation of IOUtil.readResourceAsString() took 940 ms. After the performance optimization it takes 15 ms.
You are right the BreadcrumbRenderer is the next target for performance optimizations. Parsing a XML specification takes 176 ms - which is done a second time only for BreadcrumRenderer.createBreadcrumElement(). Without this performance penalty, Concordion.NET 1.5 would be faster than the previous version 1.3 :-)
Thank you for your support to change the Announcer.
Yes, performance optimizations should be done based on measurements. I have used a profiler to identify spots for performance improvement.
One single Concordion.NET test takes about 600 - 900 ms after the proposed performance optimizations.
The call of Announcer took 1460 ms according to profiler data. The list based solution needs 5 ms.
The classLoader.getResourceAsStream() based implemenation of IOUtil.readResourceAsString() took 940 ms. After the performance optimization it takes 15 ms.
You are right the BreadcrumbRenderer is the next target for performance optimizations. Parsing a XML specification takes 176 ms - which is done a second time only for BreadcrumRenderer.createBreadcrumElement(). Without this performance penalty, Concordion.NET 1.5 would be faster than the previous version 1.3 :-)
Tim Wright
Feb 1, 2016, 1:37:32 PM2/1/16
to j.r., concordion-dev
These speed improvements will likely help the Java concordion as well.
--
You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.
To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/740282fa-47d6-4bbf-8934-6de0753d095b%40googlegroups.com.
Nigel Charman
Feb 1, 2016, 1:39:32 PM2/1/16
to concord...@googlegroups.com
That's great to see. Thanks for sharing
this information!
--
You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.
To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/740282fa-47d6-4bbf-8934-6de0753d095b%40googlegroups.com.
Reply all
Reply to author
Forward
0 new messages