Ticket #5287 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Remove explicit copy constructor & assignment operator from DateAndTime

Reported by: Russell Taylor Owned by: Russell Taylor
Priority: major Milestone: Release 2.2
Component: Mantid Keywords:
Cc: Blocked By:
Blocking: Tester: Martyn Gigg

Description

The introduction of an explicit copy assignment operator and copy constructor to the DateAndTime class had a noticeable (negative) effect on a number of the performance tests - in particular those relating to event lists. Presumably the compiler was carrying out some kind of optimisation that putting the functions in put a stop to. Since they're not doing anything special, remove them.

Attachments

DataObjectsTest.EventListTestPerformance.test_sort_tof.runtime.v.revision.ALL.png (35.5 KB) - added by Russell Taylor 8 years ago.
Performance test change example

Change History

comment:1 Changed 8 years ago by Russell Taylor

  • Status changed from new to accepted

comment:2 Changed 8 years ago by Russell Taylor

Remove 'explicit' from DateAndTime multi-argument constructors.

Re #5287. Putting 'explicit' on multi-argument constructors is meaningless (unless there are default arguments). N.B. We need conversions on our single-argument constructors in this class so don't want to put 'explicit' on those.

Changeset: 550912951de8a1a617feca0fd5ca1a3619a1d2e4

comment:3 Changed 8 years ago by Russell Taylor

  • Status changed from accepted to verify
  • Resolution set to fixed

Forgot to reference this ticket in the commit that fixed it:

Revert "Refs #5139. Added explicit assignment operator to DateAndTime."

This reverts commit 1a7774023a3abda53808d0f51cdb195722ef8ae9.

The addition of these special methods had a negative impact on a number of performance tests. Presumably they put a stop to some optimisation that the compiler was able to do with the implicitly generated versions.

Changeset: 1894bf932ce7398b58ecaa7e987daa83869d8d79

Changed 8 years ago by Russell Taylor

Performance test change example

comment:4 Changed 8 years ago by Russell Taylor

The attached image shows an example of the effect this had on a performance test. The step up and then down again, on the right of the graph, is when these methods were added and then removed.

comment:5 Changed 8 years ago by Martyn Gigg

  • Status changed from verify to verifying
  • Tester set to Martyn Gigg

comment:6 Changed 8 years ago by Martyn Gigg

  • Status changed from verifying to closed

comment:7 Changed 8 years ago by Jose Borreguero

Refs #5287 Build GridDomain

Changeset: 3b883c49d3e95d9267b7b7e81dbca9d5a1fe2f2a

comment:8 Changed 8 years ago by Jose Borreguero

Refs #5287 Build GridDomain

Changeset: 3b883c49d3e95d9267b7b7e81dbca9d5a1fe2f2a

comment:9 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6133

Note: See TracTickets for help on using tickets.