fix incorrect usage of TClonesArray::GetEntries() instead of ::GetEntriesFast()

The name of the TClonesArray::GetEntries() method is very confusing.

It does not return the array size, as one would expect. Instead, it returns the number of non-zero entries in the array.

To get the array size one should use TClonesArray::GetEntriesFast().

A bad thing about GetEntries() is that it takes O(N) operations since at each call it rolls over the entire array.

For example, a typical code:

for( int i=0; i < fMCTracks->GetEntries(); i++){

...

}

takes O(N^2) operations, because at each step GetEntries() performs an additional loop over the array.

I fixed all the places in CbmRoot where I think GetEntriesFast() must be used.

changed the description

Dear @n.herrmann, @i.deppner, @i.selyuzhenkov, @praisig, @ebechtel_AT_ikf.uni-frankfurt.de, @karpushkin_AT_inr.ru, @v.singhal, @f.uhlig, @v.friese, @p.-a.loizeau, @s.lebedev, @se.gorbunov, @v.akishina, @d.smith,

you have been identified as code owner of at least one file which was changed with this merge request.

Please check the changes and approve them or request changes.

added CodeOwners label

approved this merge request

approved this merge request

Hi @se.gorbunov, thanks for the effort! (Edit:) - The two PaPa files I will have to check more carefully (my current overview of the code is not that detailed by heart), but may be here @ebechtel_AT_ikf.uni-frankfurt.de can still comment ( in principle he is on vacation).

• CbmTrdHitProducer see the more detailed comment directly at the file.

• CbmTrdElectronsTrainAnn I think it might contain a similar issue like the change in the HitProducer.

• CbmTrdParModGas should be fine. Since, during the tokenization no empty entries should be created.

• The rest is mainly legacy code, part of it will also be dropped with some updates I am currently working on. So please go ahead there. I will try to remove obsolete stuff step by step, but for the time being I think the adjustments should enter the code.

changed the description

resolved all threads

Hi Sergey, The changes are fine from the RICH side.

Dear Sergey,

All the changes ok for MUCH, as we usually use to assert the pointer taken from the TClonesArray.

Thanks for this finding. Indeed, I used to think, that why there are 2 functions (GetEntries() and GetEntriesFast()) for the same purpose. Now it is clear.

Vikas

@se.gorbunov,

since most of the code owners approved the MR I would like to merge ist. Could you please rebase or do you allow that I rebase your branch myself.

@se.gorbunov,

I think you have to do the rebase locally and push the changes back to the GitLab server.

added 29 commits

• 5db52e97...c43cb3b3 - 27 commits from branch computing:master
• 21b2c9c8 - fix incorrect usage of TClonesArray::GetEntries() instead of ::GetEntriesFast()
• 28320484 - code format

Compare with previous version

enabled an automatic merge when the pipeline for 28320484 succeeds

merged