RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass

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

RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass

Ioi Lam-2
This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.

[1]  Change calls like

SystemDictionary::Object_klass()
SystemDictionary::Throwable_klass_is_loaded()
SystemDictionary::box_klass_type()

to

vmClasses::Object_klass()
vmClasses::Throwable_klass_is_loaded()
vmClasses::box_klass_type()

[2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.

[3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.

Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%

Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

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

Commit messages:
 - 8260471: Change SystemDictionary::xxx_klass() calls to vmClasses::xxx_klass()

Changes: https://git.openjdk.java.net/jdk/pull/2301/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2301&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260471
  Stats: 691 lines in 185 files changed: 85 ins; 64 del; 542 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2301.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2301/head:pull/2301

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass

Ioi Lam-2
On Thu, 28 Jan 2021 21:00:04 GMT, Ioi Lam <[hidden email]> wrote:

> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.

To make reviewing the 185 files a little easier, I haven't changed the copyright dates yet. I'll do that before the final integration.

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

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v2]

Ioi Lam-2
In reply to this post by Ioi Lam-2
> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>
> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>
> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>   grep -v '^[+-][+-][+-]' | grep '^[+-]'

Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:

  added missing #include systemDictionary.hpp

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2301/files
  - new: https://git.openjdk.java.net/jdk/pull/2301/files/e2f77252..e1a09411

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

  Stats: 8 lines in 8 files changed: 8 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2301.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2301/head:pull/2301

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v2]

Lois Foltan-3
On Fri, 29 Jan 2021 14:26:03 GMT, Ioi Lam <[hidden email]> wrote:

>> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>>
>> [1]  Change calls like
>>
>> SystemDictionary::Object_klass()
>> SystemDictionary::Throwable_klass_is_loaded()
>> SystemDictionary::box_klass_type()
>>
>> to
>>
>> vmClasses::Object_klass()
>> vmClasses::Throwable_klass_is_loaded()
>> vmClasses::box_klass_type()
>>
>> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>>
>> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>>
>> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>>
>> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>>
>> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>>
>> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>>   grep -v '^[+-][+-][+-]' | grep '^[+-]'
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>
>   added missing #include systemDictionary.hpp

Looks good!!
Lois

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

Marked as reviewed by lfoltan (Reviewer).

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v2]

Thomas Stuefe
On Mon, 1 Feb 2021 17:09:42 GMT, Lois Foltan <[hidden email]> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   added missing #include systemDictionary.hpp
>
> Looks good!!
> Lois

Hi Ioi,

Sorry, I still get build errors with this (linux x64, fastdebug, nopch):



   link_klass(SystemDictionary::Reference_klass());
                                ^~~~~~~~~~~~~~~

Last Commit:
commit e1a09411c12cdd95bf1f8896100284e0697fafb7 (HEAD -> pull/2301)                                                                                                   │
Author: iklam <[hidden email]>                                                                                                                                    │
Date:   Fri Jan 29 06:22:52 2021 -0800                                                                                                                                │
                                                                                                                                                                      │
    added missing #include systemDictionary.hpp

I'm also confused why no GA ran for this pr. I gave the patch a cursory read, it reads all okay but of course I cannot see from reading the patch whether we could miss some includes. I'd like to see the GA builds being successful, in this case also for the side platforms and minimal builds/zero.

Thanks, Thomas

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

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v3]

Ioi Lam-2
In reply to this post by Ioi Lam-2
> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>
> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>
> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>   grep -v '^[+-][+-][+-]' | grep '^[+-]'

Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:

  fixed AOT build

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2301/files
  - new: https://git.openjdk.java.net/jdk/pull/2301/files/e1a09411..8df077d4

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

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2301.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2301/head:pull/2301

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v2]

Ioi Lam-2
In reply to this post by Thomas Stuefe
On Mon, 1 Feb 2021 18:10:02 GMT, Thomas Stuefe <[hidden email]> wrote:

>> Looks good!!
>> Lois
>
> Hi Ioi,
>
> Sorry, I still get build errors with this (linux x64, fastdebug, nopch):
>
>
>
>    link_klass(SystemDictionary::Reference_klass());
>                                 ^~~~~~~~~~~~~~~
>
> Last Commit:
> commit e1a09411c12cdd95bf1f8896100284e0697fafb7 (HEAD -> pull/2301)                                                                                                   │
> Author: iklam <[hidden email]>                                                                                                                                    │
> Date:   Fri Jan 29 06:22:52 2021 -0800                                                                                                                                │
>                                                                                                                                                                       │
>     added missing #include systemDictionary.hpp
>
> I'm also confused why no GA ran for this pr. I gave the patch a cursory read, it reads all okay but of course I cannot see from reading the patch whether we could miss some includes. I'd like to see the GA builds being successful, in this case also for the side platforms and minimal builds/zero.
>
> Thanks, Thomas

> Hi Ioi,
>
> Sorry, I still get build errors with this (linux x64, fastdebug, nopch):
>
> ```
> /shared/projects/openjdk/jdk-jdk/source/src/hotspot/share/aot/aotCodeHeap.cpp: In member function 'void AOTCodeHeap::link_known_klasses()':
> /shared/projects/openjdk/jdk-jdk/source/src/hotspot/share/aot/aotCodeHeap.cpp:397:32: error: 'Reference_klass' is not a member of 'SystemDictionary'
>    link_klass(SystemDictionary::Reference_klass());
>                                 ^~~~~~~~~~~~~~~
> ```
>
> Last Commit:
>
> ```
> commit e1a09411c12cdd95bf1f8896100284e0697fafb7 (HEAD -> pull/2301)                                                                                                   │
> Author: iklam <[hidden email]>                                                                                                                                    │
> Date:   Fri Jan 29 06:22:52 2021 -0800                                                                                                                                │
>                                                                                                                                                                       │
>     added missing #include systemDictionary.hpp
> ```
>
> I'm also confused why no GA ran for this pr. I gave the patch a cursory read, it reads all okay but of course I cannot see from reading the patch whether we could miss some includes. I'd like to see the GA builds being successful, in this case also for the side platforms and minimal builds/zero.
>
> Thanks, Thomas

I disabled the GitHub Actions for my repo because they were creating too much noise. Instead, I've been testing my builds with Mach5, which has a much larger variety of builds. Unfortunately AOT is disabled by default in Mach5 (due to JDK-8255616: Removal of experimental features AOT and Graal JIT).

Now I've added AOT to my local builds to make sure I don't break it unintentionally. I am also re-enabling GitHub actions on my repo.

I verified that AOT builds again with [8df077d](https://github.com/openjdk/jdk/pull/2301/commits/8df077d47201dbb88171c8137acb57d26f0dd007)

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

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v3]

Harold Seigel-2
In reply to this post by Ioi Lam-2
On Mon, 1 Feb 2021 19:04:05 GMT, Ioi Lam <[hidden email]> wrote:

>> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>>
>> [1]  Change calls like
>>
>> SystemDictionary::Object_klass()
>> SystemDictionary::Throwable_klass_is_loaded()
>> SystemDictionary::box_klass_type()
>>
>> to
>>
>> vmClasses::Object_klass()
>> vmClasses::Throwable_klass_is_loaded()
>> vmClasses::box_klass_type()
>>
>> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>>
>> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>>
>> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>>
>> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>>
>> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>>
>> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>>   grep -v '^[+-][+-][+-]' | grep '^[+-]'
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>
>   fixed AOT build

Changes look good!
Thanks, Harold

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

Marked as reviewed by hseigel (Reviewer).

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v4]

Ioi Lam-2
In reply to this post by Ioi Lam-2
> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>
> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>
> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>   grep -v '^[+-][+-][+-]' | grep '^[+-]'

Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:

 - fixed comments
 - Merge branch 'master' into 8260471-SystemDictionary-to-vmClasses-rename
 - fixed AOT build
 - added missing #include systemDictionary.hpp
 - 8260471: Change SystemDictionary::xxx_klass() calls to vmClasses::xxx_klass()

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

Changes: https://git.openjdk.java.net/jdk/pull/2301/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2301&range=03
  Stats: 705 lines in 190 files changed: 94 ins; 67 del; 544 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2301.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2301/head:pull/2301

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v5]

Ioi Lam-2
In reply to this post by Ioi Lam-2
> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>
> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>
> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>   grep -v '^[+-][+-][+-]' | grep '^[+-]'

Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:

  fixed SA (serviceability/sa/CDSJMapClstats.java)

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2301/files
  - new: https://git.openjdk.java.net/jdk/pull/2301/files/533615be..7489267b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2301&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2301&range=03-04

  Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2301.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2301/head:pull/2301

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v5]

David Holmes-2
On Mon, 1 Feb 2021 20:43:01 GMT, Ioi Lam <[hidden email]> wrote:

>> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>>
>> [1]  Change calls like
>>
>> SystemDictionary::Object_klass()
>> SystemDictionary::Throwable_klass_is_loaded()
>> SystemDictionary::box_klass_type()
>>
>> to
>>
>> vmClasses::Object_klass()
>> vmClasses::Throwable_klass_is_loaded()
>> vmClasses::box_klass_type()
>>
>> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>>
>> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>>
>> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>>
>> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>>
>> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>>
>> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>>   grep -v '^[+-][+-][+-]' | grep '^[+-]'
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>
>   fixed SA (serviceability/sa/CDSJMapClstats.java)

LGTM!

Thanks,
David

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

Marked as reviewed by dholmes (Reviewer).

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v5]

Thomas Stuefe
In reply to this post by Ioi Lam-2
On Mon, 1 Feb 2021 20:43:01 GMT, Ioi Lam <[hidden email]> wrote:

>> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>>
>> [1]  Change calls like
>>
>> SystemDictionary::Object_klass()
>> SystemDictionary::Throwable_klass_is_loaded()
>> SystemDictionary::box_klass_type()
>>
>> to
>>
>> vmClasses::Object_klass()
>> vmClasses::Throwable_klass_is_loaded()
>> vmClasses::box_klass_type()
>>
>> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>>
>> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>>
>> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>>
>> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>>
>> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>>
>> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>>   grep -v '^[+-][+-][+-]' | grep '^[+-]'
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>
>   fixed SA (serviceability/sa/CDSJMapClstats.java)

Marked as reviewed by stuefe (Reviewer).

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

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v5]

Thomas Stuefe
In reply to this post by David Holmes-2
On Tue, 2 Feb 2021 07:38:37 GMT, David Holmes <[hidden email]> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   fixed SA (serviceability/sa/CDSJMapClstats.java)
>
> LGTM!
>
> Thanks,
> David

Looks good to me too. Local build worked for me now. I see GA builds went through too. Thanks for this work! Anything giving us faster builds is appreciated, since build time keeps creeping up.

..Thomas

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

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v6]

Ioi Lam-2
In reply to this post by Ioi Lam-2
> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>
> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>
> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>   grep -v '^[+-][+-][+-]' | grep '^[+-]'

Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:

 - updated copyright
 - Merge branch 'master' into 8260471-SystemDictionary-to-vmClasses-rename
 - fixed SA (serviceability/sa/CDSJMapClstats.java)
 - fixed comments
 - Merge branch 'master' into 8260471-SystemDictionary-to-vmClasses-rename
 - fixed AOT build
 - added missing #include systemDictionary.hpp
 - 8260471: Change SystemDictionary::xxx_klass() calls to vmClasses::xxx_klass()

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

Changes: https://git.openjdk.java.net/jdk/pull/2301/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2301&range=05
  Stats: 806 lines in 191 files changed: 94 ins; 67 del; 645 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2301.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2301/head:pull/2301

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

Re: RFR: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass [v5]

Ioi Lam-2
In reply to this post by Thomas Stuefe
On Tue, 2 Feb 2021 07:50:45 GMT, Thomas Stuefe <[hidden email]> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   fixed SA (serviceability/sa/CDSJMapClstats.java)
>
> Marked as reviewed by stuefe (Reviewer).

Thanks @tstuefe, @hseigel, @dholmes-ora and @lfoltan for the review. I will merge/test/push tonight when the repo is relatively quiet.

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

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

Integrated: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass

Ioi Lam-2
In reply to this post by Ioi Lam-2
On Thu, 28 Jan 2021 21:00:04 GMT, Ioi Lam <[hidden email]> wrote:

> This is the second step of https://github.com/openjdk/jdk/pull/2246 (8260467: Move well-known classes from systemDictionary.hpp to vmClasses.hpp). These are mostly boiler-plate changes done by scripts.
>
> [1]  Change calls like
>
> SystemDictionary::Object_klass()
> SystemDictionary::Throwable_klass_is_loaded()
> SystemDictionary::box_klass_type()
>
> to
>
> vmClasses::Object_klass()
> vmClasses::Throwable_klass_is_loaded()
> vmClasses::box_klass_type()
>
> [2] Remove unnecessary inclusion of systemDictionary.hpp (replace with vmClasses.hpp if necessary). In some cases, I have to add signature.hpp to some files, which only indirectly included signature.hpp through systemDictionary.hpp.
>
> [3] In the previous PR, I incorrectly used the enum name `VMClassID`. This PR changes it to `vmClassID` to match the existing use of `vmSymbolID` and `vmIntrinsicID`.
>
> Due to the refactoring of these two PRs, the number of HotSpot .o files that include systemDictionary.hpp decreases from 491 to 91. HotSpot build time is reduced by about 2%
>
> Tested with mach5: tier1, builds-tier2, builds-tier3, builds-tier4 and builds-tier5. Also locally: aarch64, arm, ppc64, s390, x86, and zero.
>
> Review Notes: if you don't want to scroll through 185 files, you may want to try:
>
> curl https://github.com/openjdk/jdk/compare/1de3c554477497d1ceee573180940e8d38c364ee...e2f77252c8b3edd4d0071cfc014290568a16de9d.diff | \
>   grep -v '^[+-][+-][+-]' | grep '^[+-]'

This pull request has now been integrated.

Changeset: ffbcf1b0
Author:    Ioi Lam <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/ffbcf1b0
Stats:     806 lines in 191 files changed: 94 ins; 67 del; 645 mod

8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass

Reviewed-by: lfoltan, hseigel, dholmes, stuefe

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

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