RFR: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

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

RFR: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Ioi Lam-2
compilerDefinitions.hpp is a popular header, included by 588 of about 1000 HotSpot .o files. It includes arguments.hpp only for a single function that's not used in critical paths.

After refactoring, the number of .o files that includes arguments.hpp reduces from 623 to 68.

(I also removed arguments.hpp from other files that don't actually need it.)

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

Commit messages:
 - 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Changes: https://git.openjdk.java.net/jdk/pull/3351/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3351&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264748
  Stats: 59 lines in 41 files changed: 16 ins; 33 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3351.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3351/head:pull/3351

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

Re: RFR: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Gerard Ziemski-2
On Tue, 6 Apr 2021 07:27:36 GMT, Ioi Lam <[hidden email]> wrote:

> compilerDefinitions.hpp is a popular header, included by 588 of about 1000 HotSpot .o files. It includes arguments.hpp only for a single function that's not used in critical paths.
>
> After refactoring, the number of .o files that includes arguments.hpp reduces from 623 to 68.
>
> (I also removed arguments.hpp from other files that don't actually need it.)

Nice!

How do you figure out where to remove the unneeded header and/or what to replace it with?

Do you use some tool to help you out with this?

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

Marked as reviewed by gziemski (Committer).

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

Re: RFR: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Ioi Lam-2
On Tue, 6 Apr 2021 19:04:57 GMT, Gerard Ziemski <[hidden email]> wrote:

> Nice!
>
> How do you figure out where to remove the unneeded header and/or what to replace it with?
>
> Do you use some tool to help you out with this?

Hi Gerard, thanks for the review. The tools I used are here: https://github.com/iklam/tools/tree/main/headers

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

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

Re: RFR: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Stefan Karlsson-3
In reply to this post by Ioi Lam-2
On Tue, 6 Apr 2021 07:27:36 GMT, Ioi Lam <[hidden email]> wrote:

> compilerDefinitions.hpp is a popular header, included by 588 of about 1000 HotSpot .o files. It includes arguments.hpp only for a single function that's not used in critical paths.
>
> After refactoring, the number of .o files that includes arguments.hpp reduces from 623 to 68.
>
> (I also removed arguments.hpp from other files that don't actually need it.)

Marked as reviewed by stefank (Reviewer).

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

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

Integrated: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Ioi Lam-2
In reply to this post by Ioi Lam-2
On Tue, 6 Apr 2021 07:27:36 GMT, Ioi Lam <[hidden email]> wrote:

> compilerDefinitions.hpp is a popular header, included by 588 of about 1000 HotSpot .o files. It includes arguments.hpp only for a single function that's not used in critical paths.
>
> After refactoring, the number of .o files that includes arguments.hpp reduces from 623 to 68.
>
> (I also removed arguments.hpp from other files that don't actually need it.)

This pull request has now been integrated.

Changeset: 17202c89
Author:    Ioi Lam <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/17202c89
Stats:     59 lines in 41 files changed: 16 ins; 33 del; 10 mod

8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Reviewed-by: gziemski, stefank

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

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

Re: RFR: 8264748: Do not include arguments.hpp from compilerDefinitions.hpp

Ioi Lam-2
In reply to this post by Stefan Karlsson-3
On Tue, 6 Apr 2021 19:54:03 GMT, Stefan Karlsson <[hidden email]> wrote:

>> compilerDefinitions.hpp is a popular header, included by 588 of about 1000 HotSpot .o files. It includes arguments.hpp only for a single function that's not used in critical paths.
>>
>> After refactoring, the number of .o files that includes arguments.hpp reduces from 623 to 68.
>>
>> (I also removed arguments.hpp from other files that don't actually need it.)
>
> Marked as reviewed by stefank (Reviewer).

Thanks @stefank and @gerard-ziemski for the review.

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

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