Removed use of static in DefaultMeasure::UpdateAxesFast #9
Conversation
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for branch cms/v1.020. @cmsbuild, @smuzaffar, @Degano, @iahmad-khan, @davidlange6 can you please review it and eventually sign? Thanks. external issue cms-sw/cmsdist#2025 |
|
@smuzaffar @davidlange6 I'm testing now to be sure this solves all the remaining problems with Njettiness |
|
The test shows that this does fix the problem: step3 of workflow 50208.0 always fails without this change and succeeds with the change. |
| // some storage, declared static to save allocation/re-allocation costs | ||
| static LightLikeAxis new_axes[N]; | ||
| static fastjet::PseudoJet new_jets[N]; | ||
| // some storage, declared on stack to save allocation/re-allocation costs |
There was a problem hiding this comment.
I don't see how this comment makes sense anymore. The arrays will be constructed and destroyed every time this method is called, so there will be some performance cost compared to declaring them static. The change is necessary for thread-safety, but the comment should be deleted.
There was a problem hiding this comment.
agree, comment now does not make any sense.
@Dr15Jones , can you update it and then I will merge it for 23h00 IB.
There was a problem hiding this comment.
I had interpreted the comment to be relative to new and delete. Looks to me like the following for loop could also be removed now.
There was a problem hiding this comment.
yes make sense to remove the for loop
The use of static arrays in DefaultMeasure::UpdateAxesFast caused thread-safety problems. The stated reason for using the arrays was to avoid allocation/deallocation but that was also achievable by having the arrays just be on the stack.
Dr15Jones
force-pushed
the
removeStaticDefaultMeasure
branch
from
03b7b81 to
e93521f
Compare
|
Pull request #9 was updated. external issue cms-sw/cmsdist#2025 |
|
I have made the suggested changes |
|
I ran my test after the code change and everything still works. |
Removed use of static in DefaultMeasure::UpdateAxesFast
The use of static arrays in DefaultMeasure::UpdateAxesFast caused
thread-safety problems. The stated reason for using the arrays
was to avoid allocation/deallocation but that was also achievable
by having the arrays just be on the stack.