ursjoss opened a new pull request #438:
URL: https://github.com/apache/wicket/pull/438


   While migrating my project (see 
[PR](https://github.com/ursjoss/scipamato/pull/177) there) from the mocking 
library [mockito](https://site.mockito.org/) to [mock](https://mockk.io/), I 
ran into weird performance issues. The build time jumped from roughly 20 
minutes to eventually more than 4 hours. I took some effort improving various 
things in my own code (less mocking, more memory etc.) but did not bring it 
down considerably until I addressed a bottleneck I found in wicket.
   
   `AjaxRequestHandler` maintains a collection of listeners in a `LinkedList`. 
When client code adds a new listener calling method `addListener`, it uses the 
collection's method `contains` to assert we don't add a listener to the 
collection twice. This doesn't scale well. For some reason, my setup with mockk 
seems to add many more listeners than what my code did with mockito, so I ran 
into this issue with continuously slower test executions.
   
   I managed to temporarily resolve the problem in my project by plugging in my 
own copy of 
[AjaxRequestHandler](https://github.com/ursjoss/scipamato/blob/master/core/core-web/src/test/kotlin/ch/difty/scipamato/TestAjaxRequestHandler.kt#L101)
 where I swapped the collection type from LinkedList to LinkedHashSet (calling 
`setAjaxRequestTargetProvider` in my 
[TestApplication](https://github.com/ursjoss/scipamato/blob/master/core/core-web/src/test/kotlin/ch/difty/scipamato/TestApplication.kt#L29)).
 With this approach the total build time went down again to 20-30 minutes.
   
   This PR serves the purpose of starting a discussion about whether this 
change would be feasible to implement in wicket directly. To me, it is not 
apparent why a LinkedList had been chosen initially. The LinkedHashSet performs 
much better with contains and still preserves insertion order. If that latter 
should turn out to be not relevant, we could even consider using a HashSet 
instead of a linked HashSet, but I presume insertion order is important.
   
   A separate topic on my side will be to figure out why my setup with mockito 
seemed to work well while migrating to mockk surfaced this issue.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to