RFR: 8253219: Epsilon: clean up unnecessary includes
Static analysis shows a few #include statements in gc/epsilon that are not needed. Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH} ------------- Commit messages: - 8253219: Epsilon: clean up unnecessary includes Changes: https://git.openjdk.java.net/jdk/pull/198/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=198&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253219 Stats: 3 lines in 2 files changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/198.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/198/head:pull/198 PR: https://git.openjdk.java.net/jdk/pull/198
Hi, On 16.09.20 09:49, Aleksey Shipilev wrote:
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
- in epsilonMonitoringSupport.cpp, the include for allocation.inline.hpp is correct, CHeapObj is declared there. The allocation.hpp one is superfluous though. Looks good otherwise. I do not need a re-review for this change. Thomas
On 9/16/20 12:52 PM, Thomas Schatzl wrote:
Hi,
On 16.09.20 09:49, Aleksey Shipilev wrote:
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
- in epsilonMonitoringSupport.cpp, the include for allocation.inline.hpp is correct, CHeapObj is declared there. The allocation.hpp one is superfluous though.
Seems vice versa: allocation.hpp defines CHeapObj and needs to be included, and allocation.inline.hpp needs to be dropped. Updated. -- Thanks, -Aleksey
Hi, On 16.09.20 13:00, Aleksey Shipilev wrote:
On 9/16/20 12:52 PM, Thomas Schatzl wrote:
Hi,
On 16.09.20 09:49, Aleksey Shipilev wrote:
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
- in epsilonMonitoringSupport.cpp, the include for allocation.inline.hpp is correct, CHeapObj is declared there. The allocation.hpp one is superfluous though.
Seems vice versa: allocation.hpp defines CHeapObj and needs to be included, and allocation.inline.hpp needs to be dropped. Updated.
I tend to always prefer the .inline.hpp if available as something might be defined in the .inline.hpp that is used in the .hpp file. I.e. as a rule include to .inline.hpp if available. It does not seem to be the case here so just the .hpp should be good. Looks good. Thomas
On Wed, 16 Sep 2020 07:43:31 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
Marked as reviewed by tschatzl (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/198
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: Reinstate allocation.hpp include ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/198/files - new: https://git.openjdk.java.net/jdk/pull/198/files/f6db93d1..fb49b3de Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=198&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=198&range=00-01 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/198.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/198/head:pull/198 PR: https://git.openjdk.java.net/jdk/pull/198
On Wed, 16 Sep 2020 11:05:26 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
Reinstate allocation.hpp include
Marked as reviewed by kbarrett (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/198
On Wed, 16 Sep 2020 11:12:04 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
Reinstate allocation.hpp include
Marked as reviewed by kbarrett (Reviewer).
_Mailing list message from [Thomas Schatzl](mailto:thomas.schatzl@oracle.com) on [hotspot-gc-dev](mailto:hotspot-gc-dev@openjdk.java.net):_ I tend to always prefer the .inline.hpp if available as something might be defined in the .inline.hpp that is used in the .hpp file. I.e. as a rule include to .inline.hpp if available. It does not seem to be the case here so just the .hpp should be good.
Right. I do as crystal^W static analyzer guides: include only what is necessary. Integrating now. ------------- PR: https://git.openjdk.java.net/jdk/pull/198
On Wed, 16 Sep 2020 07:43:31 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
Static analysis shows a few #include statements in gc/epsilon that are not needed.
Testing: Linux x86_64 {release, fastdebug, slowdebug} x {+PCH, -PCH}
This pull request has now been integrated. Changeset: f509eb06 Author: Aleksey Shipilev <shade@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/f509eb06 Stats: 2 lines in 2 files changed: 2 ins; 0 del; 0 mod 8253219: Epsilon: clean up unnecessary includes Reviewed-by: tschatzl, kbarrett ------------- PR: https://git.openjdk.java.net/jdk/pull/198
participants (5)
-
Aleksey Shipilev
-
Aleksey Shipilev
-
Kim Barrett
-
Thomas Schatzl
-
Thomas Schatzl