Hi Caleb, On Mar 3, 2010, at 7:36 AM, Caleb James DeLisle wrote:
> As I see it there are three known workarounds for the problem at the > moment. > 1. Isolate the tests - <forkMode>pertest</forkMode> > 2. Clear the context (patch). > 3. Increase the memory for tests - <argLine>-Xmx512m</argLine> * What about 2? Does 2 require increased memory too? * BTW we all agree about reducing build times. It is *critical* to have the fastest possible build. * I'm really surprised that the time difference is so important. I remember I had done some tests when I put the forktest and the differences were very small which is why we did it (maybe we added a lot of tests since then, causing the problem). * BTW2: We could easily have the fork only in xwiki-core and have all other build modules use non forked tests. * I don't understand why 1 would take more memory than 3. * Personally, as I've already said, I'm for 2 if it solves the memory issue and allows to run tests in non fork mode. As a second option I'm for 3. Thanks -Vincent > > Here is a test with the memory increased: > [INFO] Total time: 13 minutes 41 seconds > [INFO] Finished at: Wed Mar 03 00:20:01 GMT-05:00 2010 > [INFO] Final Memory: 96M/218M > > And with the tests forked (current setup) > [INFO] Total time: 34 minutes 58 seconds > [INFO] Finished at: Wed Mar 03 01:02:29 GMT-05:00 2010 > [INFO] Final Memory: 167M/496M > > both built from platform/trunks with mvn clean install -Pdev > > I think the options listed above are (IMO) comparable in terms of > 'hackishness' with the only difference that one takes almost three > times as long to compile. > > It seems that the viewpoints on this are: > 1. Cob it together for now and hope the leaks get fixed or retired > with the old core. > 2. Make it take a long time to compile in the hope that someone will > get disgusted enough to route out some of the leaks. > > I favor 1 because the old core is deprecated so those leaks' days > are numbered anyway, and also when platform takes a half hour to > build, contributors will find it harder to make and test patches. > > Ok, I will stop beating this dead horse now. > Caleb > > > Vincent Massol wrote: >> On Mar 1, 2010, at 11:42 AM, Caleb James DeLisle wrote: >> >>> >>> Vincent Massol wrote: >>>> On Mar 1, 2010, at 11:18 AM, Caleb James DeLisle wrote: >>>> >>>>> Vincent Massol wrote: >>>>>> On Mar 1, 2010, at 10:48 AM, Caleb James DeLisle wrote: >>>>>> >>>>>>> By clearing the XWikiContext in the tearDown of the unit test, I can >>>>>>> stop the memory leak and stop running the tests in isolation. I am >>>>>>> proposing we apply this change because the XWikiContext is created >>>>>>> per test and the worst problem which can be created is tests >>>>>>> erroneously passing which I don't think is very likely from a change >>>>>>> like this. >>>>>> +1 for the main reason that this improves build times. The second >>>>>> reason is that it makes memory leaks more visible which gives us a >>>>>> better chance to find them (although this won't replace performance >>>>>> tests running over a few days for ex). >>>>>> >>>>>> However please add a comment just above the getContext().clear() to >>>>>> explain why this is needed. BTW why is it needed? :) >>>>> I will do that. The reason it's needed is because (it seems) the >>>>> context is referenced by a static variable which isn't cleared and >>>>> rather than pick through the tests looking for where the reference >>>>> was (and hoping no test ever does it again) I opted to clear the >>>>> context and let the tests use the context as they wish. This is my >>>>> understanding of the situation. >>>> Then the question is why do we reference the context statically? Can't we >>>> remove that? >>> Static referencing is more a theory than a fact, the problem as I >>> see it is the context is used everywhere and referenced in a lot of >>> places so anything which is able to escape the tearDown process and >>> happens to hold a reference to the context will cause references to >>> persist pointing to everything. I think the choice is to either >>> clear the context or pour through each test individually looking for >>> any reference which might escape tearDown. I remember running >>> various tests individually and in pairs with very little memory and >>> finding a lot of them seem to have leaks. >> >> Well the GC doesn't run all the time so you need to ensure you're forcing it >> to run after a test to know if the test has a leak. >> >> And yes, I couldn't get to the bottom of it in the past (didn't spend enough >> time) which is why I took the easy route of forking tests to isolate them. >> >> Thanks >> -Vincent >> >>>>>>> Jira issue: http://jira.xwiki.org/jira/browse/XWIKI-4953 >>>>>>> Patch: >>>>>>> http://jira.xwiki.org/jira/secure/attachment/16788/XWIKI-4496-patchTestMemoryLeak.patch >>>>>>> >>>>>>> Patch content: >>>>>>> >>>>>>> Index: core/xwiki-core/pom.xml >>>>>>> =================================================================== >>>>>>> --- core/xwiki-core/pom.xml (revision 27357) >>>>>>> +++ core/xwiki-core/pom.xml (working copy) >>>>>>> @@ -823,10 +823,6 @@ >>>>>>> <plugin> >>>>>>> <groupId>org.apache.maven.plugins</groupId> >>>>>>> <artifactId>maven-surefire-plugin</artifactId> >>>>>>> - <configuration> >>>>>>> - <!-- Prevent Out Of Memory errors resulting from tests >>>>>>> that do no free up the memory correctly --> >>>>>>> - <forkMode>pertest</forkMode> >>>>>>> - </configuration> >>>>>>> </plugin> >>>>>>> <!-- Publish a Core Test JAR --> >>>>>>> <plugin> >>>>>>> Index: >>>>>>> core/xwiki-core/src/test/java/com/xpn/xwiki/test/AbstractBridgedXWikiComponentTestCase.java >>>>>>> =================================================================== >>>>>>> --- >>>>>>> core/xwiki-core/src/test/java/com/xpn/xwiki/test/AbstractBridgedXWikiComponentTestCase.java >>>>>>> (revision 27357) >>>>>>> +++ >>>>>>> core/xwiki-core/src/test/java/com/xpn/xwiki/test/AbstractBridgedXWikiComponentTestCase.java >>>>>>> (working copy) >>>>>>> @@ -80,6 +80,7 @@ >>>>>>> protected void tearDown() throws Exception >>>>>>> { >>>>>>> Utils.setComponentManager(null); >>>>>>> + getContext().clear(); >>>>>>> super.tearDown(); >>>>>>> } _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
