[Brainstorming] Loss of TPC from 2017-11-09 to 2018-04-03

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[Brainstorming] Loss of TPC from 2017-11-09 to 2018-04-03

vmassol
Administrator
Hi,

I’ve posted the following on matrix/IRC.

Friday:

* Guys we regressed in global TPC from  2017-11-09 to 2018-04-03 :(
* From 76.28% to 75.88%
* we need to analyze more in details now to understand why
* (and whether the report is correct or not)
* PDF Report: https://up1.xwikisas.com/#mJ0loeB6nBrAgYeKA7MGGw
* Examples:
** com.xpn.xwiki.internal.store from 97.6% to 89.32%
** com.xpn.xwiki.internal.file from 67.9% to 9.87%  - that's huge maybe some refactoring
** org.xwiki.url.internal from 86.99% to 63.69%

Today:

* Note that I have 2 reasons for checking our TPC:
** Globally it has to go up every year and we've gone down since nov 2017
** For STAMP we need to show an increase in TPC and we had a nice +2.2% increase from 2016 to 2017 but we've gone down now and we need to go up, to go to +3 or/ 4% at end of 2018

Today I’ve also started analyzing the drop from  67.9% to 9.87% for “com.xpn.xwiki.internal.file”. Here are my findings:
* This is caused by TemporaryDeferredFileRepository which is no longer tested
** Before: http://maven.xwiki.org/site/clover/20171109/clover-commons+rendering+platform-20171109-1920/com/xpn/xwiki/internal/file/pkg-summary.html
** After; http://maven.xwiki.org/site/clover/20180403/clover-commons+rendering+platform-20180403-1915/com/xpn/xwiki/internal/file/pkg-summary.html
* There have been refactoring recently to enable FS attachment store by default that have led to TemporaryDeferredFileRepository not being used anymore (deprecated methods not used anymore) or used only in some specific circumstances (for example at https://github.com/xwiki/xwiki-platform/blob/ac85d61b0c48d7ed21a8109e964f77e7502b93fe/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateRecycleBinStore.java#L230when the store is not defined)
* The conclusion is that before the changesTemporaryDeferredFileRepository was called a lot (indirectly) leading to 545 calls to it (http://maven.xwiki.org/site/clover/20171109/clover-commons+rendering+platform-20171109-1920/com/xpn/xwiki/internal/file/TemporaryDeferredFileRepository.html?line=74#src-74) and now it’s not called anymore at all.
* The second conclusion is that we had no unit/integration tests for it and the changes lead to loosing it being tested. So it’s no longer tested ATM. Thanks to the Clover report we noticed that it’s no longer tested.
* Thus we need to do one of 2 things:
** Move the code to legacy and make sure no code uses it. That’s if we don’t really need it
** Keep it but add some unit/integration tests or even some functional tests that would trigger it.

I’ll let Thomas pitch in on this.

Next step: analyze some other drops in TPC.

Note: I wish it were simpler to find out why some TPC drops. Took me like 1-2 hours to figure all this out...

Thanks
-Vincent



Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] Loss of TPC from 2017-11-09 to 2018-04-03

vmassol
Administrator


> On 16 Apr 2018, at 16:00, Vincent Massol <[hidden email]> wrote:
>
> Hi,
>
> I’ve posted the following on matrix/IRC.
>
> Friday:
>
> * Guys we regressed in global TPC from  2017-11-09 to 2018-04-03 :(
> * From 76.28% to 75.88%
> * we need to analyze more in details now to understand why
> * (and whether the report is correct or not)
> * PDF Report: https://up1.xwikisas.com/#mJ0loeB6nBrAgYeKA7MGGw
> * Examples:
> ** com.xpn.xwiki.internal.store from 97.6% to 89.32%
> ** com.xpn.xwiki.internal.file from 67.9% to 9.87%  - that's huge maybe some refactoring
> ** org.xwiki.url.internal from 86.99% to 63.69%
>
> Today:
>
> * Note that I have 2 reasons for checking our TPC:
> ** Globally it has to go up every year and we've gone down since nov 2017
> ** For STAMP we need to show an increase in TPC and we had a nice +2.2% increase from 2016 to 2017 but we've gone down now and we need to go up, to go to +3 or/ 4% at end of 2018
>
> Today I’ve also started analyzing the drop from  67.9% to 9.87% for “com.xpn.xwiki.internal.file”. Here are my findings:
> * This is caused by TemporaryDeferredFileRepository which is no longer tested
> ** Before: http://maven.xwiki.org/site/clover/20171109/clover-commons+rendering+platform-20171109-1920/com/xpn/xwiki/internal/file/pkg-summary.html
> ** After; http://maven.xwiki.org/site/clover/20180403/clover-commons+rendering+platform-20180403-1915/com/xpn/xwiki/internal/file/pkg-summary.html
> * There have been refactoring recently to enable FS attachment store by default that have led to TemporaryDeferredFileRepository not being used anymore (deprecated methods not used anymore) or used only in some specific circumstances (for example at https://github.com/xwiki/xwiki-platform/blob/ac85d61b0c48d7ed21a8109e964f77e7502b93fe/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateRecycleBinStore.java#L230when the store is not defined)
> * The conclusion is that before the changesTemporaryDeferredFileRepository was called a lot (indirectly) leading to 545 calls to it (http://maven.xwiki.org/site/clover/20171109/clover-commons+rendering+platform-20171109-1920/com/xpn/xwiki/internal/file/TemporaryDeferredFileRepository.html?line=74#src-74) and now it’s not called anymore at all.
> * The second conclusion is that we had no unit/integration tests for it and the changes lead to loosing it being tested. So it’s no longer tested ATM. Thanks to the Clover report we noticed that it’s no longer tested.
> * Thus we need to do one of 2 things:
> ** Move the code to legacy and make sure no code uses it. That’s if we don’t really need it
> ** Keep it but add some unit/integration tests or even some functional tests that would trigger it.
>
> I’ll let Thomas pitch in on this.
>
> Next step: analyze some other drops in TPC.

Analysis 2 (a simple one):

Two new package were added since 2017-11-09:
* org.xwiki.job.handler.internal
* org.xwiki.job.handler.internal.question

And they have very low TPC: 2..9% for the former and 0% for the later!

This package has 5 classes on master (3 in 20180403):
* http://maven.xwiki.org/site/clover/20180403/clover-commons+rendering+platform-20180403-1915/org/xwiki/job/handler/internal/pkg-summary.html
* http://maven.xwiki.org/site/clover/20180403/clover-commons+rendering+platform-20180403-1915/org/xwiki/job/handler/internal/question/pkg-summary.html

So this means we have very little or nothing testing the code in this package.

Thus it means we need to write some tests for this.

I guess this is for Thomas. WDYT Thomas?

Thanks
-Vincent


>
> Note: I wish it were simpler to find out why some TPC drops. Took me like 1-2 hours to figure all this out...
>
> Thanks
> -Vincent
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [Brainstorming] Loss of TPC from 2017-11-09 to 2018-04-03

vmassol
Administrator


> On 16 Apr 2018, at 18:19, Vincent Massol <[hidden email]> wrote:
>
>
>
>> On 16 Apr 2018, at 16:00, Vincent Massol <[hidden email]> wrote:
>>
>> Hi,
>>
>> I’ve posted the following on matrix/IRC.
>>
>> Friday:
>>
>> * Guys we regressed in global TPC from  2017-11-09 to 2018-04-03 :(
>> * From 76.28% to 75.88%
>> * we need to analyze more in details now to understand why
>> * (and whether the report is correct or not)
>> * PDF Report: https://up1.xwikisas.com/#mJ0loeB6nBrAgYeKA7MGGw
>> * Examples:
>> ** com.xpn.xwiki.internal.store from 97.6% to 89.32%
>> ** com.xpn.xwiki.internal.file from 67.9% to 9.87%  - that's huge maybe some refactoring
>> ** org.xwiki.url.internal from 86.99% to 63.69%
>>
>> Today:
>>
>> * Note that I have 2 reasons for checking our TPC:
>> ** Globally it has to go up every year and we've gone down since nov 2017
>> ** For STAMP we need to show an increase in TPC and we had a nice +2.2% increase from 2016 to 2017 but we've gone down now and we need to go up, to go to +3 or/ 4% at end of 2018

[snip]

Analysis 3: We should add some unit tests for org.xwiki.resource.events classes. Right now there’s no tests and they’re tested indirectly only. That gets them to 52% but it’s a bit low for new codfe (new code and easy to test like this should get at least 80% coverage). For ex here it would be very easy to add some tests for hashcode/equals.

See http://maven.xwiki.org/site/clover/20180403/clover-commons+rendering+platform-20180403-1915/org/xwiki/resource/events/pkg-summary.html

I’ll let Thomas comment since this is some code he added.

Thanks
-Vincent