2 doesn't require any increased memory, just clearing the context in the teardown.
Vincent Massol wrote: > 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. Great, I guess I was just feeling cynical when i thought "make it slow and someone will fix it". > * 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). I always remember 1/2 hour, maybe it's my computer. > * BTW2: We could easily have the fork only in xwiki-core and have all other > build modules use non forked tests. I did a find-grep and xwiki-core is the only place I find 'pertest' > * I don't understand why 1 would take more memory than 3. Not sure either, one guess is starting and stopping the vm. > * 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. "solves" might be too strong a word :) but it runs with the default memory settings. I have to build again, I'll try the clear context patch and report the time building that. Thanks for the interest, Caleb > > 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 > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
