RFR: 8261672: Reduce inclusion of classLoaderData.hpp

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8261672: Reduce inclusion of classLoaderData.hpp

Ioi Lam-2
classLoaderData.hpp is included by about 700 out of 1000 .o files in HotSpot. Most of these are transitively included through klass.hpp, typeArrayKlass.hpp and instanceKlass.hpp.

These headers can be refactored by moving inline functions that depend on ClassLoaderData to xxx.inline.hpp. This reduces the .o files that include classLoaderData.hpp to about 260.

(I also removed a bunch of unnecessary inclusion of classLoader.hpp from a few C files).
 
Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

-------------

Commit messages:
 - 8261672: Reduce inclusion of classLoaderData.hpp
 - reduce classLoader.hpp

Changes: https://git.openjdk.java.net/jdk/pull/2555/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2555&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261672
  Stats: 160 lines in 59 files changed: 108 ins; 47 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2555.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2555/head:pull/2555

PR: https://git.openjdk.java.net/jdk/pull/2555
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261672: Reduce inclusion of classLoaderData.hpp

Lois Foltan-3
On Fri, 12 Feb 2021 19:41:54 GMT, Ioi Lam <[hidden email]> wrote:

> classLoaderData.hpp is included by about 700 out of 1000 .o files in HotSpot. Most of these are transitively included through klass.hpp, typeArrayKlass.hpp and instanceKlass.hpp.
>
> These headers can be refactored by moving inline functions that depend on ClassLoaderData to xxx.inline.hpp. This reduces the .o files that include classLoaderData.hpp to about 260.
>
> (I also removed a bunch of unnecessary inclusion of classLoader.hpp from a few C files).
>  
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

LGTM.
Lois

-------------

Marked as reviewed by lfoltan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2555
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261672: Reduce inclusion of classLoaderData.hpp

Coleen Phillimore-3
In reply to this post by Ioi Lam-2
On Fri, 12 Feb 2021 19:41:54 GMT, Ioi Lam <[hidden email]> wrote:

> classLoaderData.hpp is included by about 700 out of 1000 .o files in HotSpot. Most of these are transitively included through klass.hpp, typeArrayKlass.hpp and instanceKlass.hpp.
>
> These headers can be refactored by moving inline functions that depend on ClassLoaderData to xxx.inline.hpp. This reduces the .o files that include classLoaderData.hpp to about 260.
>
> (I also removed a bunch of unnecessary inclusion of classLoader.hpp from a few C files).
>  
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

Looks good.  I think adding compiledICHolder.inline.hpp is worth doing because performance matters to its caller in compiledMethod.cpp.

-------------

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2555
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261672: Reduce inclusion of classLoaderData.hpp [v2]

Ioi Lam-2
In reply to this post by Ioi Lam-2
> classLoaderData.hpp is included by about 700 out of 1000 .o files in HotSpot. Most of these are transitively included through klass.hpp, typeArrayKlass.hpp and instanceKlass.hpp.
>
> These headers can be refactored by moving inline functions that depend on ClassLoaderData to xxx.inline.hpp. This reduces the .o files that include classLoaderData.hpp to about 260.
>
> (I also removed a bunch of unnecessary inclusion of classLoader.hpp from a few C files).
>  
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:

 - fixed copyright
 - Merge branch 'master' into 8261672-reduce-classLoaderData.hpp
 - 8261672: Reduce inclusion of classLoaderData.hpp
 - reduce classLoader.hpp

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2555/files
  - new: https://git.openjdk.java.net/jdk/pull/2555/files/96f3b680..ea396957

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2555&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2555&range=00-01

  Stats: 4352 lines in 164 files changed: 2494 ins; 690 del; 1168 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2555.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2555/head:pull/2555

PR: https://git.openjdk.java.net/jdk/pull/2555
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261672: Reduce inclusion of classLoaderData.hpp [v2]

Ioi Lam-2
In reply to this post by Coleen Phillimore-3
On Fri, 12 Feb 2021 21:35:38 GMT, Coleen Phillimore <[hidden email]> wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>>
>>  - fixed copyright
>>  - Merge branch 'master' into 8261672-reduce-classLoaderData.hpp
>>  - 8261672: Reduce inclusion of classLoaderData.hpp
>>  - reduce classLoader.hpp
>
> Looks good.  I think adding compiledICHolder.inline.hpp is worth doing because performance matters to its caller in compiledMethod.cpp.

Thanks @coleenp and @lfoltan for the review.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2555
Reply | Threaded
Open this post in threaded view
|

Integrated: 8261672: Reduce inclusion of classLoaderData.hpp

Ioi Lam-2
In reply to this post by Ioi Lam-2
On Fri, 12 Feb 2021 19:41:54 GMT, Ioi Lam <[hidden email]> wrote:

> classLoaderData.hpp is included by about 700 out of 1000 .o files in HotSpot. Most of these are transitively included through klass.hpp, typeArrayKlass.hpp and instanceKlass.hpp.
>
> These headers can be refactored by moving inline functions that depend on ClassLoaderData to xxx.inline.hpp. This reduces the .o files that include classLoaderData.hpp to about 260.
>
> (I also removed a bunch of unnecessary inclusion of classLoader.hpp from a few C files).
>  
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

This pull request has now been integrated.

Changeset: 235da6aa
Author:    Ioi Lam <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/235da6aa
Stats:     182 lines in 59 files changed: 108 ins; 47 del; 27 mod

8261672: Reduce inclusion of classLoaderData.hpp

Reviewed-by: lfoltan, coleenp

-------------

PR: https://git.openjdk.java.net/jdk/pull/2555