Ticket #8784 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

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:1 Changed 7 years ago by Martyn Gigg

  • Milestone changed from Release 3.1 to Release 3.2

comment:2 Changed 7 years ago by Martyn Gigg

  • Status changed from new to inprogress

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:5 Changed 7 years ago by Peter Peterson

  • Blocking 6995 added

comment:6 Changed 7 years ago by Martyn Gigg

  • Blocking 8983 added

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

Tidyup the Logger headers.

Refs #8784

Changeset: 99b95f0ffeeb9d7000125a536c57914bb75ebc9e

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

Add required headers to StepScan

Refs #8784

Changeset: 77b6ecf2e1438993225a65265c2d282893ebbc5d

comment:17 Changed 7 years ago by Martyn Gigg

Add required includes to tests.

Refs #8784

Changeset: 238ac1efb806fad91d57eeaa4723516e329b19d9

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

Include cmath for RHEL6 build.

Refs #8784

Changeset: 074bc95dc3bffc2e6f80897000d716038ca4253e

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

Tidyup the Logger headers.

Refs #8784

Changeset: 99b95f0ffeeb9d7000125a536c57914bb75ebc9e

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

Add required headers to StepScan

Refs #8784

Changeset: 77b6ecf2e1438993225a65265c2d282893ebbc5d

comment:31 Changed 7 years ago by Martyn Gigg

Add required includes to tests.

Refs #8784

Changeset: 238ac1efb806fad91d57eeaa4723516e329b19d9

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

Include cmath for RHEL6 build.

Refs #8784

Changeset: 074bc95dc3bffc2e6f80897000d716038ca4253e

comment:35 Changed 7 years ago by Martyn Gigg

Export declarations for Windows.

Refs #8784

Changeset: aa668cff7a500feede088ce66f57150a53187a77

comment:36 Changed 7 years ago by Martyn Gigg

Poco header for windows.

Refs #8784

Changeset: de365e875d49c4aa1dfc6d93abd6a2ef0074152a

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

Note: See TracTickets for help on using tickets.