Ticket #8784 (closed: fixed)
Remove unrequired includes in some core headers
Reported by: | Martyn Gigg | Owned by: | Martyn Gigg |
---|---|---|---|
Priority: | major | Milestone: | Release 3.2 |
Component: | Framework | Keywords: | Maintenance |
Cc: | Blocked By: | ||
Blocking: | #6995, #8983 | Tester: | Peter Parker |
Description
I had cause to fiddle with cow_ptr and was surprised to find that half of Geometry started recompiling. I'm sure there are many other places that include many things that could be forward declared.
This will be a time-boxed ticket to examine Kernel/Geometry/API code to remove unnecessary includes.
Change History
comment:3 Changed 7 years ago by Martyn Gigg
- Keywords Maintenance, Cleanup added; Maintenance removed
comment:4 Changed 7 years ago by Martyn Gigg
- Keywords Maintenance added; Maintenance, Cleanup removed
comment:7 Changed 7 years ago by Martyn Gigg
Tidy up Instrument.h
Some users of Instrument now require a couple of extra headers. Refs #8784
Changeset: 6d5c348679df5801ff3c3b1886368445c8798314
comment:8 Changed 7 years ago by Martyn Gigg
Remove cow_ptr from VectorHelper header.
Refs #8784
Changeset: d2aa81bceaf1ec293698333be36f0df454c16100
comment:9 Changed 7 years ago by Martyn Gigg
Tidy up the ReadLock/WriteLock header includes
Uses forward declarations where possible. Refs #8784
Changeset: 38e1529a48f451856842d81322d7dff90be225f2
comment:10 Changed 7 years ago by Martyn Gigg
Prune headers in the Kernel Property tree.
Many things here could be forward declared so that the headers don't propagate through the code. The MaskedProperty implementations have been moved to the cpp file to help with this. Refs #8784
Changeset: 8e5b100d2ec0204cf6ba7268bfdd1f948cd532c9
comment:11 Changed 7 years ago by Martyn Gigg
Fix cpp files requiring the Detector.h header
Refs #8784
Changeset: 86f31ee34d7a2cb1af15d799e39b02190c9534ba
comment:12 Changed 7 years ago by Martyn Gigg
comment:13 Changed 7 years ago by Martyn Gigg
Tidyup Algorithm.h header
Mainly removed Poco includes and used forward declarations instead. Unsuprisingly this has affected some algorithms. Refs #8784
Changeset: 33b062dd9ac1edc3b9e30b0151301378789f53a9
comment:14 Changed 7 years ago by Martyn Gigg
Thin out the Progress & ProgressBase headers.
Implement ProgressBase copy constructor & assignment operator now required as the constructor allocates memory. Refs #8784
Changeset: fb77ed9256b41b981dd0a0bccbb3b245321e9ae9
comment:15 Changed 7 years ago by Martyn Gigg
Further header removals from PropertyManager
Refs #8784
Changeset: 0909e5ac55179223a6f4d290c44767ebea82acf7
comment:16 Changed 7 years ago by Martyn Gigg
comment:17 Changed 7 years ago by Martyn Gigg
comment:18 Changed 7 years ago by Martyn Gigg
Add required Kernel headers for other platforms.
Refs #8784
Changeset: 4c52b460862d4fbb6508de127683d0172736c1a2
comment:19 Changed 7 years ago by Martyn Gigg
Fixes for includes required in a release build.
Refs #8784
Changeset: 059260224f5c2ccd0c4d2355c038a279cd672158
comment:20 Changed 7 years ago by Martyn Gigg
comment:21 Changed 7 years ago by Martyn Gigg
Tidy up Instrument.h
Some users of Instrument now require a couple of extra headers. Refs #8784
Changeset: 6d5c348679df5801ff3c3b1886368445c8798314
comment:22 Changed 7 years ago by Martyn Gigg
Remove cow_ptr from VectorHelper header.
Refs #8784
Changeset: d2aa81bceaf1ec293698333be36f0df454c16100
comment:23 Changed 7 years ago by Martyn Gigg
Tidy up the ReadLock/WriteLock header includes
Uses forward declarations where possible. Refs #8784
Changeset: 38e1529a48f451856842d81322d7dff90be225f2
comment:24 Changed 7 years ago by Martyn Gigg
Prune headers in the Kernel Property tree.
Many things here could be forward declared so that the headers don't propagate through the code. The MaskedProperty implementations have been moved to the cpp file to help with this. Refs #8784
Changeset: 8e5b100d2ec0204cf6ba7268bfdd1f948cd532c9
comment:25 Changed 7 years ago by Martyn Gigg
Fix cpp files requiring the Detector.h header
Refs #8784
Changeset: 86f31ee34d7a2cb1af15d799e39b02190c9534ba
comment:26 Changed 7 years ago by Martyn Gigg
comment:27 Changed 7 years ago by Martyn Gigg
Tidyup Algorithm.h header
Mainly removed Poco includes and used forward declarations instead. Unsuprisingly this has affected some algorithms. Refs #8784
Changeset: 33b062dd9ac1edc3b9e30b0151301378789f53a9
comment:28 Changed 7 years ago by Martyn Gigg
Thin out the Progress & ProgressBase headers.
Implement ProgressBase copy constructor & assignment operator now required as the constructor allocates memory. Refs #8784
Changeset: fb77ed9256b41b981dd0a0bccbb3b245321e9ae9
comment:29 Changed 7 years ago by Martyn Gigg
Further header removals from PropertyManager
Refs #8784
Changeset: 0909e5ac55179223a6f4d290c44767ebea82acf7
comment:30 Changed 7 years ago by Martyn Gigg
comment:31 Changed 7 years ago by Martyn Gigg
comment:32 Changed 7 years ago by Martyn Gigg
Add required Kernel headers for other platforms.
Refs #8784
Changeset: 4c52b460862d4fbb6508de127683d0172736c1a2
comment:33 Changed 7 years ago by Martyn Gigg
Fixes for includes required in a release build.
Refs #8784
Changeset: 059260224f5c2ccd0c4d2355c038a279cd672158
comment:34 Changed 7 years ago by Martyn Gigg
comment:35 Changed 7 years ago by Martyn Gigg
comment:36 Changed 7 years ago by Martyn Gigg
comment:37 Changed 7 years ago by Martyn Gigg
Delay instantiation of algorithm notification objects
If an algorithm is never run it is a bit of a waste to have things created and destroyed when registering them. Refs #8784
Changeset: 87c1faca088821e8d77ee0f9c14dd76aaaa446c6
comment:38 Changed 7 years ago by Martyn Gigg
Kill a compiler warning in NormaliseToMonitorTest
Refs #8784
Changeset: bdd82176b6afd1f249561643eb4bfd1be836f878
comment:39 Changed 7 years ago by Russell Taylor
The Jenkins valgrind job, which was previously clean for Kernel, is complaining about a leak in ProgressBase: https://builds.sns.gov/view/Static%20Analysis/job/ornl_valgrind_develop/209/valgrindResult/pid=10966,0x0/
From inspection it's not obvious what the problem is - any ideas?
comment:40 Changed 7 years ago by Martyn Gigg
No I'm not immediately sure either. It seems to suggest a problem with the copy/assignment operator but I think it looks okay.
I'll run it locally and try and investigate.
comment:41 Changed 7 years ago by Martyn Gigg
Fix bug in ProgressBase copy constructor/assignment operator
Refs #8784
Changeset: 253df817ff06c8b36a8588d64837f6a1b0654ca9
comment:42 Changed 7 years ago by Martyn Gigg
- Status changed from inprogress to verify
- Resolution set to fixed
Branch: bugfix/8784_unneeded_headers
Tester: This could be an endless exercise so I'm stopping at the algorithm/property hierarchy as that should have the most benefit. It has in fact reduced the clean build compile times by between 5-8 minutes. There are many other places, i.e. tests that include too many headers where we could probably reduce the compile time even more but that will have to wait for another day.
This should produce *NO* visible changes within the running code itself, testing will have to be done by code review. The unit/systemtests should all be passing and please ensure I have fixed the valgrind error in comment 39.
comment:43 Changed 7 years ago by Martyn Gigg
Note that removing the workspace_iterator code was spun out to #8983 as there were more changes than I was comfortable making here.
comment:44 Changed 7 years ago by Karl Palmen
- Status changed from verify to verifying
- Tester set to Karl Palmen
comment:45 Changed 7 years ago by Karl Palmen
Kernel test errors have gone from Valgrind build 210.
comment:46 Changed 7 years ago by Karl Palmen
Build failed with unresolved external errors associated with time series. I am now rebuilding in the hope this works.
comment:47 Changed 7 years ago by Karl Palmen
- Status changed from verifying to reopened
- Resolution fixed deleted
Build is still failing for me.
May be all that is needed are instructions to build after this change has gone into master.
comment:48 Changed 7 years ago by Martyn Gigg
- Status changed from reopened to verify
- Resolution set to fixed
I have tried this on another couple of machines and it's okay without anything special.
comment:49 Changed 7 years ago by Peter Parker
- Status changed from verify to verifying
- Tester changed from Karl Palmen to Peter Parker
comment:50 Changed 7 years ago by Peter Parker
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/bugfix/8784_unneeded_headers'
Full changeset: 17d4d628fd6c9458cf4c91b14b24684d1209dbee
comment:51 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9628