Event comparison performance hit

[Russell Taylor]

Changes in #7945 has a marked effect on a whole bunch of performance tests, and even showed up in some system tests (examples attached).

The cause of this needs to be found and reversed.

[Peter Peterson]

Re #8056. Making internal helper methods private.

Changeset: f571a4b3b20b6fed2d3c59ba5e35a41f79657c5d

[Peter Peterson]

Re #8056. Making one helper function inline.

Changeset: 7e911d4da78caa863e197a767d558a3d6a7105a7

[Russell Taylor]

I note that this last change recovered half the time previously 'lost' in EventListTestPerformance.test_compressEvents, but no change is observed in other affected tests such as EventListTestPerformance.test_integrate or RebinByPulseTimesTestPerformance.testExecution

[Russell Taylor]

The full list of affected performance tests is:

• AlgorithmsTest.CropWorkspaceTestPerformance.test_crop_events_inplace
• AlgorithmsTest.DiffractionFocussing2TestPerformance.test_SNAP_event_one_group_dontPreserveEvents
• AlgorithmsTest.FilterByXValueTestPerformance.test_crop_events_inplace
• AlgorithmsTest.RebinByPulseTimesTestPerformance.testExecution
• DataObjectsTest.EventListTestPerformance.test_compressEvents
• DataObjectsTest.EventListTestPerformance.test_compressEvents_Parallel
• DataObjectsTest.EventListTestPerformance.test_integrate
• DataObjectsTest.EventListTestPerformance.test_sort_tof

[Nick Draper]

Am I right in saying that this change only really affects the speed of CheckWorkspacesMatch? If so I think we should:

1. move this ticket to the next release
2. Remove the time taken to compare results from the performance tests
3. Add one performance test just for testing the speed of CheckWorkspacesMatch.

[Russell Taylor]

Re #8056. Inlined a number of methods for performance.

These methods were previously (implicitly) inlined. Moving their definitions out to the cpp had a negative impact on performance. I've moved the definitions back to the header file, but now they are explicitly inlined and defined outside of the class declaration.

Changeset: 370ae01444135f8ff0d778a039c3404a121b643e

[Russell Taylor]

All the affected tests listed above have recovered their performance (look ​here to see the DataObjects ones). There's one that we don't know about yet: DiffractionFocussing2TestPerformance only showed a change on the ISIS jobs, which is Ubuntu rather than RHEL but which looks at master instead of develop.

To test (branch is bugfix/8056_event_performance), look at the changes and see that they were just moving code, not changing it.

Note that none of this has anything to do with CheckWorkspacesMatch - it was just that they slowdown was introduced in a ticket that was doing stuff for that algorithm.

[Peter Peterson]

Merge remote branch 'origin/bugfix/8056_event_performance'

Full changeset: ​56272a5876a7d60f3c7d7d0fb1b53382ccd564c5

[Stuart Campbell]

This ticket has been transferred to github issue ​8901