New subject: [xwiki-devs] [Proposal] clear XWikiContext between core unit tests to plug memory leak and stop running tests in isolation

1 Mar 2010

After running some more tests I find that 13 and 34 minutes were
both outliers. I found on Hudson that the last 10 builds averaged
16.1 minutes each, I would like to make this change and then revisit
the issue after the next 10 builds.

WDYT?

Caleb

Some more statistics:

With the tests forked (current setup)

[INFO] Total time: 27 minutes 37 seconds
[INFO] Finished at: Thu Mar 04 11:22:56 GMT-05:00 2010
[INFO] Final Memory: 169M/496M

[INFO] Total time: 23 minutes 33 seconds
[INFO] Finished at: Thu Mar 04 09:10:04 GMT-05:00 2010
[INFO] Final Memory: 173M/496M

[INFO] Total time: 21 minutes 22 seconds
[INFO] Finished at: Thu Mar 04 13:31:52 GMT-05:00 2010
[INFO] Final Memory: 188M/496M

>>> [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

With patch for clearing context:
[INFO] Total time: 20 minutes 45 seconds
[INFO] Finished at: Wed Mar 03 03:44:52 GMT-05:00 2010
[INFO] Final Memory: 182M/496M

[INFO] Total time: 20 minutes 32 seconds
[INFO] Finished at: Sat Mar 06 00:16:52 GMT-05:00 2010
[INFO] Final Memory: 193M/496M

With memory increased.
[INFO] Total time: 22 minutes 59 seconds
[INFO] Finished at: Wed Mar 03 06:19:45 GMT-05:00 2010
[INFO] Final Memory: 191M/496M

>>> [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

Caleb James DeLisle wrote:
> 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
> 

[xwiki-devs] [Proposal] clear XWikiContext between core unit tests to plug memory leak and stop running tests in isolation

Caleb James DeLisle

Vincent Massol

Caleb James DeLisle

Vincent Massol

Thomas Mortagne

Caleb James DeLisle

Vincent Massol

Caleb James DeLisle

Vincent Massol

Caleb James DeLisle

Caleb James DeLisle

Vincent Massol

Thomas Mortagne

tags

participants (3)