Ticket #5954 (closed: fixed)
Fix Instrument Loading Performance
| Reported by: | Owen Arnold | Owned by: | Owen Arnold |
|---|---|---|---|
| Priority: | critical | Milestone: | Release 2.3 |
| Component: | Mantid | Keywords: | |
| Cc: | taylorrj@… | Blocked By: | |
| Blocking: | Tester: | Russell Taylor |
Description
Changes made under #5900 are functionally fine, but may have adversely affected the performance in this critical region.
Run under a profiler and try to fix whatever is causing the issue, or revert the code.
Attachments
Change History
comment:2 Changed 8 years ago by Owen Arnold
refs #5954. Add PefTest to parser.
Allows me to check how bad things are and benchmark for improvement. Can also use this for callgrind improvement refactorings Changeset: 5f65aea2595889364ab97d07466731963581df1b
comment:3 Changed 8 years ago by Owen Arnold
refs #5954. Cache paths.
According to callgrind Peforming Poco::PATH::path() turns out to be very expensive. Peform it once and cache the results.
Changeset: fb6e193efbba163ecfa7d1a514b29ea87bb018e5
comment:5 Changed 8 years ago by Owen Arnold
Based on the changes above to IDFObject, I've reduced the execution time of the new performance tests from 1.06 seconds to 0.41 seconds.
comment:6 Changed 8 years ago by Owen Arnold
refs #5954. Small performance improvement
Changeset: 3f868263c1ab76eccbc57c4bb554123733de313d
comment:7 Changed 8 years ago by Owen Arnold
The changes implemented thus far as part of this ticket have caused quite a significant improvement in performance, but we're not yet back to the baseline (i.e. speed prior to #5900). I hope to achieve speeds roughly similar to the original, although, I doubt that I can actually improve on the original speed.
comment:8 Changed 8 years ago by Owen Arnold
refs #5954. Smarter construction.
By moving to an interface and the Null Object pattern, we do not need to construct empty paths.
Changeset: c06f5d2a9f4d8208eeacbf3039d07698f9607fde
comment:9 Changed 8 years ago by Owen Arnold
From the last commit:
09:47:29 Congratulations! You sped up the performance of test DataHandlingTest.LoadInstrumentTestPerformance.test_CNCS 09:47:29 (was 5.373 s, now 3.757 s. Speed changed by +43.0 %.) 09:47:29 Congratulations! You sped up the performance of test DataHandlingTest.LoadInstrumentTestPerformance.test_SEQUOIA 09:47:29 (was 13.605 s, now 9.838 s. Speed changed by +38.3 %.) 09:47:29 Congratulations! You sped up the performance of test DataHandlingTest.LoadInstrumentTestPerformance.test_WISH 09:47:29 (was 9.190 s, now 6.992 s. Speed changed by +31.4 %.)
comment:10 Changed 8 years ago by Owen Arnold
- Status changed from accepted to verify
- Resolution set to fixed
Tester: From the performance tests Jenkins job, looks like I've recovered the baseline performance. See attached.
It may still be possible to improve the performance further, by looking at inlining the methods on this class.
comment:11 Changed 8 years ago by Russell Taylor
- Status changed from verify to verifying
- Tester set to Russell Taylor
comment:12 Changed 8 years ago by Russell Taylor
- Status changed from verifying to closed
I agree that the performance of the affected tests has been recovered.
comment:13 Changed 8 years ago by Owen Arnold
refs #5954. Add PefTest to parser.
Allows me to check how bad things are and benchmark for improvement. Can also use this for callgrind improvement refactorings Changeset: 5f65aea2595889364ab97d07466731963581df1b
comment:14 Changed 8 years ago by Owen Arnold
refs #5954. Cache paths.
According to callgrind Peforming Poco::PATH::path() turns out to be very expensive. Peform it once and cache the results.
Changeset: fb6e193efbba163ecfa7d1a514b29ea87bb018e5
comment:15 Changed 8 years ago by Owen Arnold
refs #5954. Small performance improvement
Changeset: 3f868263c1ab76eccbc57c4bb554123733de313d
comment:16 Changed 8 years ago by Owen Arnold
refs #5954. Smarter construction.
By moving to an interface and the Null Object pattern, we do not need to construct empty paths.
Changeset: c06f5d2a9f4d8208eeacbf3039d07698f9607fde
comment:17 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 6800
