Re: enhancing perfcheck - Proof of concept & proposals

Hi Laurent,

On Wed, 2014-10-15 at 14:26 +0200, Laurent Godard wrote:
> i'll present in the comming line the approach i've tested regarding make 
> perfcheck

That's great, thanks for working on this.

> 1- percheck tests not restricted at class level
> -----------------------------------------------
>
> The problem being at classe level is that some tests may be very long 
> and hide problems on other performed tests. Time can be doubled without 
> noticing it
>
> The goal here is to be able to DUMP callgrind each time we want it
>
> Instead of using
> CALLGRIND_DUMP_STATS
>
> one can use
> CALLGRIND_DUMP_STATS_AT(message);

The problem was with processing the data but your python script looks good,
so, yes, it's much better to use CALLGRIND_DUMP_STATS_AT.

> a first approach of a "big" test, with 3 dumps is here
> https://gerrit.libreoffice.org/#/c/11949/

I will also put some comments there,
but in general, I think it's good to keep the tests as small as possible.
This one runs 5minutes for me doing some 18bn cycles.
Most of that is just loading the file - maybe we could use 'loperf' for
testing import/export and do only the rest as perfchecks? What do you think?

If you have specific documents you would like to add for loperf, let me know.

> 2- exploiting results
> ---------------------
>
> The dumps are stored in workdir/CppunitTest/
>
> Each dump file starting with callgrind.out contains
> - the message of step 1
> - the total of cycles
>
> here is a python script that walk through workdir/CppunitTest/ and 
> retreive these 2 information for each dump file
> It also retreives the currently last commit

I like this approach :-).. a nice idea
We don't need to care about CALLGRIND_DUMP_STATS anymore,
which is really cool, thanks.

> it append these results in a csv file
> TODO: use this csv file to monitor or detect problems
>
> https://gerrit.libreoffice.org/#/c/11962/
>
> output example
>
> $ cat perfcheckResult.csv
> lastCommit      test name       filedatetime    dump comment    count
> 741c661ece19ccb4e94bb20ceb75d89a29b1b2a8        sc_perf_searchobj 
> 10/14/2014 09:54:52     testSheetFindAll - Search value 11403647297

Fun, for me Search value is more than 10x faster - Was there some fix recently?
10/22/2014 08:27:58 testSheetFindAll - Search value 766042247

> 741c661ece19ccb4e94bb20ceb75d89a29b1b2a8        sc_perf_searchobj 
> 10/14/2014 09:54:53     testSheetFindAll - Search style 767867
> 741c661ece19ccb4e94bb20ceb75d89a29b1b2a8        sc_perf_searchobj 
> 10/14/2014 09:52:06     Load Big File   17078422628

Well, this is good but it's hard to parse the results quickly.
Do you think we could have date/commit in one line with all numbers?
And descriptions somewhere at the top.
So that we could compare results in one column easily (and draw graphs..)
Something like
http://dev-builds.libreoffice.org/callgrind_report/history.fods


> 3- re use of existing tests for percheck
> ----------------------------------------
>
> i tried this approach because monitoring perfs would lead to write 
> duplicate tests

Yes, we really need to reuse them.

> the basic idea, in an existing test, would be to write something like
>
> startPerfInstrumentation();
>    uno::Reference< container::XIndexAccess > xIndex = 
> xSearchable->findAll(xSearchDescr);
>    endPerfInstrumentation("testSheetFindAll - Search value");
>
> where startPerfInstrumentation and endPerfInstrumentation do nothing if 
> not in a perfcheck context

Yep
 
> see whole code example at
> https://gerrit.libreoffice.org/#/c/11982/
>
> this context is set using$(eval $(call 
> gb_CppunitTest_add_defs,sc_perf_searchobj,\
>      -DIS_PERFCHECK \
> ))
>
> see then
> https://gerrit.libreoffice.org/#/c/11982/2/sc/qa/perf/perf_instrumentation.cxx,cm
>
> the idea is to have 2 make files
> - one for subsequent test
> - one for perfcheck (that sets IS_PERFCHECK)
>
> that point to the same source test but lets perf_instrumentation.cxx be 
> re-build each time

So - now that I think about it.
Maybe it would be better to stop duplicating makefiles too.
We would use something like ENABLE_PERFTESTS (--enable-performance-testing)
- would be cool to create reasonable name for it -
and compile everything just once based on that.

Or - even better - we could just compile in the callgrind code all the time and decide when
running make, whether we want to run under valgrind --tool=callgrind or not (or both).
If that works. :-)
So, something like IS_PERFCHECK is always true, no duplication
and only decide whether to run under valgrind.

Does that make sense?
What do you think?

> ok, i stop here, ask me if something is not clear

:-) thanks a lot.

All the best,

Matus