Issues with Valgrind documenation

1. The valgrind page seems to have some "old config" fragments .

I think we should remove those. Then the CA section does not need to be anymore separate

2. Callgrind - a call graph profiler (extended version of Cachegrind) You can use this to profile only your algorithm, using Valkyrie.

The doxygen link for Valkyrie is broken. But at large here we should prb point to the callgrind documentation

3. In the twiki we have a grid section that seems to be for release 21.0 or so. Does not seem up-to-date and not sure if used.

4. Special issue with object factory design in Tracking (this is from Valgrind examples and issues)

Not sure this is relevant. I think I know what it describes and we had to fix such issues (@amete and @sroe were involved in prb a few tickets).

But the solution given in any C++11 guide is that factory methods should return unique_ptr<object> . So this should by now be kind of best practice advice and cleaner to express as such

assigned to @tovsiann

assigned to @tovsiann

changed title from Remove olf style Vallgrind options to Remove old style Vallgrind options

changed title from Remove old style Vallgrind options to Issue with Vallgrind documenation

changed the description

changed title from Issue with Vallgrind documenation to Issues with Vallgrind documenation

changed the description

Hi @tovsiann ,

would you have time to look at this?

Cheers,

James.

changed title from Issues with Vallgrind documenation to Issues with Valgrind documenation

Is this still an issue? Moving to the new repo.

changed the description

I was looking the new documentation.

I think most things are fixed.

Just on point 4 which is still there.

[https://atlas-software.docs.cern.ch/athena/performance/valgrind-examples/]

The sections towards the end , describe pre c++11 solution (and I hope also issues). More or less one should try to have factory methods returning unique_ptr . This was the solution I , @sroe , @amete used anytime we had such a leal in the post C++11 world we now live.

Or put otherwise in the context of the example that is debugged above. on who forgot to delete what

These days we return

typedef std::vector<std::unique_ptr<Trk::TrackParameters>> TrackParametersUVector;

So except if someone loops and "releases" (in which case we know who to call...) , the issue described can not happen that easily.

So my suggestion is to actually leave the issues as are instructive . But rephrase the suggested solution to indicate how these can be solved and avoided as much as possible in C++11.

This is actually equivalent to presenting the solution suggested by our "coding" standard and review instructions

Hi @christos, that is true, the example is outdated. I didn’t change it because there’s no relevant code at the moment. I can either update the example to note that we now use TrackParametersUVector, or I can replace it with a more modern example, such as the one from our tutorial. What do you think? https://gitlab.cern.ch/tovsiann/performance-tutorial-202501/-/blob/master/ValgrindExercise.md

Tbh

Up to you

The tutorial has the same issue/solution anyhow use unique_ptr

 std::unique_ptr<CaloProtoCluster> myCluster = std::make_unique<CaloProtoCluster>(cellCollLink);

vs

 CaloProtoCluster* myCluster = new CaloProtoCluster(cellCollLink);

The extra bit in the page is that this was passed via multiple layer of calls, and encapsulation so was hard to figure who leaked it.

But since the solution is the same , follow the coding guideline for C++11 I am fine either way.

If you want to keep the old example, the point would be the same. We find the old interface of naked ptr vs the new with unique ptr and say the same as the tutorial more or less

mentioned in merge request !102 (merged)