On Thu, Dec 8, 2011 at 09:52, Vincent Massol <[email protected]> wrote:

On Dec 8, 2011, at 9:01 AM, Thomas Mortagne wrote:

...

Hi devs,

Now that master just moved to 3.4-SNAPSHOT I would like to merge my refactoring of the component manager. You can find the branch on https://github.com/xwiki/xwiki-commons/tree/feature-improvecm.

The rational is that it's then going to be indirectly tested during the whole 3.4 timeframe. Never too careful with the most critical code.

I already detailed this on another mail but the major difference with current implementation is that it's locking a lot less and since CM is pretty heavily used (and is going to be used more and more) it should make a noticeable difference. It also fix several bugs I found while doing this refactoring and covering it with tests.

Here are the related jira issues: * http://jira.xwiki.org/browse/XCOMMONS-63 * http://jira.xwiki.org/browse/XCOMMONS-65 * http://jira.xwiki.org/browse/XCOMMONS-64 * http://jira.xwiki.org/browse/XCOMMONS-66

Here is my +1

+1

How are we going to measure the performance improvements?

Do we really need to waste time on measuring that precisely ? Do you see any reason for it to be really worse ?

I'd propose that we add a performance unit test so that we can compare the 2 implementations.

I can think of at least 2 tools for this:

* ContiPerf: http://databene.org/contiperf I had written a quick minimalist test here:

http://jira.xwiki.org/browse/XWIKI-6164?focusedCommentId=59460&page=com.atla...

* Tempus-fugit: http://code.google.com/p/tempus-fugit/wiki/Documentation?tm=6

ContiPerf seems the best to me.

We wouldn't run this test as part of the main test suite but it could be either run manually or triggered by a maven profile.

WDYT?

The change mades are all favorable to a performance improvement, but also improve code quality and maintenance, so knowing precisely how much we really get does not seems important to me. I do not really see the added value for end users.

Thanks -Vincent

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO

On Thu, Dec 8, 2011 at 10:39, Vincent Massol <[email protected]> wrote:

On Dec 8, 2011, at 10:21 AM, Denis Gervalle wrote:

...

On Thu, Dec 8, 2011 at 09:52, Vincent Massol <[email protected]> wrote:

...

On Dec 8, 2011, at 9:01 AM, Thomas Mortagne wrote:

...

Hi devs,

Now that master just moved to 3.4-SNAPSHOT I would like to merge my refactoring of the component manager. You can find the branch on https://github.com/xwiki/xwiki-commons/tree/feature-improvecm.

The rational is that it's then going to be indirectly tested during the whole 3.4 timeframe. Never too careful with the most critical code.

I already detailed this on another mail but the major difference with current implementation is that it's locking a lot less and since CM is pretty heavily used (and is going to be used more and more) it should make a noticeable difference. It also fix several bugs I found while doing this refactoring and covering it with tests.

Here are the related jira issues: * http://jira.xwiki.org/browse/XCOMMONS-63 * http://jira.xwiki.org/browse/XCOMMONS-65 * http://jira.xwiki.org/browse/XCOMMONS-64 * http://jira.xwiki.org/browse/XCOMMONS-66

Here is my +1

+1

How are we going to measure the performance improvements?

Do we really need to waste time on measuring that precisely ? Do you see any reason for it to be really worse ?

Well there are several things involved here:

* verifying it works. I don't think Thomas has verified that yet (I mean in heavy multithreaded situations). Better doing it in a test than waiting for it to happen live. Also it's quite simple to write as can be testified by my example.

There is already some tests to check that it works, and if it really do not works, ok, we may learn it the hard way quickly, but we will not miss it. I have reviewed the patch, and I am really confident.

* I don't see how testing could be bad

Testing is never bad, I am just talking about priorities. By the way, you have not initially raised the idea that we may need a heavy multithreaded tests, this sounds better to me than having performance numbers for now.

* It's interesting to see if we win times. Because we've seen with a profiler that the whole time spent in ECM is about 15% of the overall time which is quite significant and yes it's good to know if we're improving this or not because if we do then we can advertise it in the release notes. I prefer knowing than just hiding my head in the sand.

I was not aware of that, this could raise the priority, but this is unrelated to the current improvement proposed by Thomas. Are you really afraid that is could be worse ?

* It provides a baseline against which we can measure how we progress now in the future

...

...

I'd propose that we add a performance unit test so that we can compare the 2 implementations.

I can think of at least 2 tools for this:

* ContiPerf: http://databene.org/contiperf I had written a quick minimalist test here:

http://jira.xwiki.org/browse/XWIKI-6164?focusedCommentId=59460&page=com.atla...

...

* Tempus-fugit: http://code.google.com/p/tempus-fugit/wiki/Documentation?tm=6

ContiPerf seems the best to me.

We wouldn't run this test as part of the main test suite but it could be either run manually or triggered by a maven profile.

WDYT?

The change mades are all favorable to a performance improvement, but also improve code quality and maintenance, so knowing precisely how much we really get does not seems important to me. I do not really see the added value for end users.

Errr?

So you're suggesting not to write tests anymore because they don't bring value to end users…. Come on, Denis, I thought you were better than that! :)

Have I said that writing tests is bad ? I have just said that having a number here, and even in the release notes, will not benefit much, only geek will understand. It is more a matter of priority IMO, a test for stabilizing a real end-user functionality is far more useful. Now, you mark a point on the testing ground, if this could ensure CM is more stable, and this could be quickly set, I would agree, but not just for the numbers.

Thanks -Vincent

...

...

Thanks -Vincent

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO