Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v4]
On Thu, 4 Apr 2024 17:31:53 GMT, Stefan Karlsson wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address next round of comments > > Looks good. Thank you, @stefank, @iwanowww, @dean-long and @xmas92 for reviews and @RealFYang and @offamitkumar for testing. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2038043339
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v4]
On Thu, 4 Apr 2024 17:04:30 GMT, Vladimir Kozlov wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Address next round of comments Looks good. - Marked as reviewed by stefank (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18554#pullrequestreview-1980693606
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v4]
> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE > [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) > which was used for AOT [JEP 295](https://openjdk.org/jeps/295) > implementation in JDK 9. The code was left in HotSpot assuming it will help > in a future. But during work on Leyden we decided to not use it. In Leyden > cached compiled code will be restored in CodeCache as normal nmethods: no > need to change VM's runtime and GC code to process them. > > I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce > header size in separate changes. In these changes I did simple fields > reordering to keep small (1 byte) fields together. > > I do not see (and not expected) performance difference with these changes. > > Tested tier1-5, xcomp, stress. Running performance testing. > > I need help with testing on platforms which Oracle does not support. Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Address next round of comments - Changes: - all: https://git.openjdk.org/jdk/pull/18554/files - new: https://git.openjdk.org/jdk/pull/18554/files/33768fb2..0c18ff17 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18554=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18554=02-03 Stats: 80 lines in 13 files changed: 0 ins; 0 del; 80 mod Patch: https://git.openjdk.org/jdk/pull/18554.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18554/head:pull/18554 PR: https://git.openjdk.org/jdk/pull/18554
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 07:54:16 GMT, Stefan Karlsson wrote: >> Vladimir Kozlov 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 five additional >> commits since the last revision: >> >> - Address comments >> - Merge branch 'master' into 8329332 >> - Removed not_used state of nmethod >> - remove trailing whitespace >> - 8329332: Remove CompiledMethod and CodeBlobLayout classes > > I took a second pass over the changes. I've given a few suggestions below. > None of them should require respinning of tests (except for making sure that > this still builds). Thank you, @stefank , for second round of review. I addressed all your comments. I also removed `virtual` from `virtual method() override;` as you suggested before since I touched these files again. I will wait result of new GHA and tier1 in mach5 before integration. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2037731476
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 16:03:12 GMT, Stefan Karlsson wrote: >> Quote: "an" goes before words that begin with vowels. > > I don't think that holds if the 'n' is pronounced the way nmethod is > pronounced. `grep` shows that we have both cases but `an nmethod` is used more. - PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1552024067
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 16:16:41 GMT, Vladimir Kozlov wrote: >> I don't think that holds if the 'n' is pronounced the way nmethod is >> pronounced. > > `grep` shows that we have both cases but `an nmethod` is used more. I will fix it here as you suggested but I am not touching other places. - PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1552025626
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 07:51:47 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov 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 five additional
>> commits since the last revision:
>>
>> - Address comments
>> - Merge branch 'master' into 8329332
>> - Removed not_used state of nmethod
>> - remove trailing whitespace
>> - 8329332: Remove CompiledMethod and CodeBlobLayout classes
>
> src/hotspot/share/code/codeBlob.hpp line 168:
>
>> 166: bool is_vtable_blob() const { return _kind ==
>> CodeBlobKind::Blob_Vtable; }
>> 167: bool is_method_handles_adapter_blob() const { return _kind ==
>> CodeBlobKind::Blob_MH_Adapter; }
>> 168: bool is_upcall_stub() const { return _kind ==
>> CodeBlobKind::Blob_Upcall; }
>
> The `Blob_` prefix is now redundant since we always have to prefix with
> CodeBlobKind::. Just a suggestion if you want to shorten these.
Good suggestion
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1552009581
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 15:56:34 GMT, Vladimir Kozlov wrote: >> src/hotspot/share/gc/shared/gcBehaviours.hpp line 31: >> >>> 29: #include "oops/oopsHierarchy.hpp" >>> 30: >>> 31: // This is the behaviour for checking if a nmethod is unloading >> >> Maybe this should be *an* nmethod? > > Quote: "an" goes before words that begin with vowels. I don't think that holds if the 'n' is pronounced the way nmethod is pronounced. - PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1552005187
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 07:31:24 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov 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 five additional
>> commits since the last revision:
>>
>> - Address comments
>> - Merge branch 'master' into 8329332
>> - Removed not_used state of nmethod
>> - remove trailing whitespace
>> - 8329332: Remove CompiledMethod and CodeBlobLayout classes
>
> src/hotspot/share/gc/x/xUnload.cpp line 78:
>
>> 76: class XIsUnloadingBehaviour : public IsUnloadingBehaviour {
>> 77: public:
>> 78: virtual bool has_dead_oop(nmethod* const nm) const {
>
> `nmethod* const nm` => `nmethod* nm`. (ZGC's style is to use const for local
> variables, but not for variables in the parameter list). The same applies to
> the rest of the changes to this file.
Okay. I did not know that it is only used for locals. I will update code.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551997411
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 07:26:21 GMT, Stefan Karlsson wrote: >> Vladimir Kozlov 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 five additional >> commits since the last revision: >> >> - Address comments >> - Merge branch 'master' into 8329332 >> - Removed not_used state of nmethod >> - remove trailing whitespace >> - 8329332: Remove CompiledMethod and CodeBlobLayout classes > > src/hotspot/share/gc/shared/gcBehaviours.hpp line 31: > >> 29: #include "oops/oopsHierarchy.hpp" >> 30: >> 31: // This is the behaviour for checking if a nmethod is unloading > > Maybe this should be *an* nmethod? Quote: "an" goes before words that begin with vowels. - PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551989120
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 00:05:20 GMT, Vladimir Kozlov wrote:
>> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE
>> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365)
>> which was used for AOT [JEP 295](https://openjdk.org/jeps/295)
>> implementation in JDK 9. The code was left in HotSpot assuming it will help
>> in a future. But during work on Leyden we decided to not use it. In Leyden
>> cached compiled code will be restored in CodeCache as normal nmethods: no
>> need to change VM's runtime and GC code to process them.
>>
>> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce
>> header size in separate changes. In these changes I did simple fields
>> reordering to keep small (1 byte) fields together.
>>
>> I do not see (and not expected) performance difference with these changes.
>>
>> Tested tier1-5, xcomp, stress. Running performance testing.
>>
>> I need help with testing on platforms which Oracle does not support.
>
> Vladimir Kozlov 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 five additional
> commits since the last revision:
>
> - Address comments
> - Merge branch 'master' into 8329332
> - Removed not_used state of nmethod
> - remove trailing whitespace
> - 8329332: Remove CompiledMethod and CodeBlobLayout classes
I took a second pass over the changes. I've given a few suggestions below. None
of them should require respinning of tests (except for making sure that this
still builds).
src/hotspot/share/code/codeBlob.hpp line 168:
> 166: bool is_vtable_blob() const { return _kind ==
> CodeBlobKind::Blob_Vtable; }
> 167: bool is_method_handles_adapter_blob() const { return _kind ==
> CodeBlobKind::Blob_MH_Adapter; }
> 168: bool is_upcall_stub() const { return _kind ==
> CodeBlobKind::Blob_Upcall; }
The `Blob_` prefix is now redundant since we always have to prefix with
CodeBlobKind::. Just a suggestion if you want to shorten these.
src/hotspot/share/gc/shared/gcBehaviours.hpp line 31:
> 29: #include "oops/oopsHierarchy.hpp"
> 30:
> 31: // This is the behaviour for checking if a nmethod is unloading
Maybe this should be *an* nmethod?
src/hotspot/share/gc/shenandoah/shenandoahUnload.cpp line 81:
> 79: class ShenandoahIsUnloadingBehaviour : public IsUnloadingBehaviour {
> 80: public:
> 81: virtual bool has_dead_oop(nmethod* const nm) const {
Is there a reason why this was changed to `nmethod* const nm` instead of
`nmethod* nm`? IsUnloadingBehviour::has_dead_oop uses `nmethod* nm`. This
question applies to the other changes in this file as well.
src/hotspot/share/gc/x/xUnload.cpp line 78:
> 76: class XIsUnloadingBehaviour : public IsUnloadingBehaviour {
> 77: public:
> 78: virtual bool has_dead_oop(nmethod* const nm) const {
`nmethod* const nm` => `nmethod* nm`. (ZGC's style is to use const for local
variables, but not for variables in the parameter list). The same applies to
the rest of the changes to this file.
src/hotspot/share/gc/z/zUnload.cpp line 77:
> 75: class ZIsUnloadingBehaviour : public IsUnloadingBehaviour {
> 76: public:
> 77: virtual bool has_dead_oop(nmethod* const nm) const {
`nmethod* const nm` => `nmethod* nm`. (ZGC's style is to use const for local
variables, but not for variables in the parameter list). The same applies to
the rest of the changes to this file.
src/hotspot/share/runtime/javaThread.hpp line 123:
> 121: DeoptResourceMark* _deopt_mark; // Holds special
> ResourceMark for deoptimization
> 122:
> 123: nmethod* _deopt_nmethod; // nmethod that is
> currently being deoptimized
The alignment is (and was) weird here.
-
Marked as reviewed by stefank (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18554#pullrequestreview-1978954058
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551107567
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551073461
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551077362
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551080101
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551080280
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1551100400
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 00:05:20 GMT, Vladimir Kozlov wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Vladimir Kozlov 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 five additional > commits since the last revision: > > - Address comments > - Merge branch 'master' into 8329332 > - Removed not_used state of nmethod > - remove trailing whitespace > - 8329332: Remove CompiledMethod and CodeBlobLayout classes There is a stale comment in `test/jdk/com/sun/jdi/EATests.java:1288` -// (See CompiledMethod::is_at_poll_return()) +// (See nmethod::is_at_poll_return()) - PR Review: https://git.openjdk.org/jdk/pull/18554#pullrequestreview-1978884423
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
On Thu, 4 Apr 2024 00:05:20 GMT, Vladimir Kozlov wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Vladimir Kozlov 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 five additional > commits since the last revision: > > - Address comments > - Merge branch 'master' into 8329332 > - Removed not_used state of nmethod > - remove trailing whitespace > - 8329332: Remove CompiledMethod and CodeBlobLayout classes GHA `linux-x64-hs-minimal` failure is not related to changes: 2024-04-04T00:07:46.9654262Z ##[warning]Failed to download action 'https://api.github.com/repos/actions/github-script/tarball/60a0d83039c74a4aee543508d2ffcb1c3799cdea'. Error: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing. 2024-04-04T00:07:46.9656929Z ##[warning]Back off 22.252 seconds before retry. 2024-04-04T00:08:52.1252710Z ##[error]The SSL connection could not be established, see inner exception. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2035859221
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v3]
> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE > [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) > which was used for AOT [JEP 295](https://openjdk.org/jeps/295) > implementation in JDK 9. The code was left in HotSpot assuming it will help > in a future. But during work on Leyden we decided to not use it. In Leyden > cached compiled code will be restored in CodeCache as normal nmethods: no > need to change VM's runtime and GC code to process them. > > I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce > header size in separate changes. In these changes I did simple fields > reordering to keep small (1 byte) fields together. > > I do not see (and not expected) performance difference with these changes. > > Tested tier1-5, xcomp, stress. Running performance testing. > > I need help with testing on platforms which Oracle does not support. Vladimir Kozlov 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 five additional commits since the last revision: - Address comments - Merge branch 'master' into 8329332 - Removed not_used state of nmethod - remove trailing whitespace - 8329332: Remove CompiledMethod and CodeBlobLayout classes - Changes: - all: https://git.openjdk.org/jdk/pull/18554/files - new: https://git.openjdk.org/jdk/pull/18554/files/246ff68a..33768fb2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18554=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18554=01-02 Stats: 9283 lines in 197 files changed: 3058 ins; 4514 del; 1711 mod Patch: https://git.openjdk.org/jdk/pull/18554.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18554/head:pull/18554 PR: https://git.openjdk.org/jdk/pull/18554
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 17:55:38 GMT, Stefan Karlsson wrote:
>> No, `CodeCache::blob_count()` uses different macro `FOR_ALL_HEAPS(heap)`
>> because it looks for all code blobs, not only nmethods.
>>
>> `CodeCache::nmethod_count()` is the only place where `FOR_ALL_NMETHOD_HEAPS
>> ` was used. So I decided to remove the macro.
>
> I didn't say that blob_count used `FOR_ALL_NMETHODS_HEAP`. I wrote "one of
> these macros". I still think this adds an inconsistency to the code that I
> don't think is beneficial.
>
> With that said, can't this be written as:
>
> for (CodeHeap* heap : *_nmethod_heaps) {
>
>
> Maybe yet another opportunity for cleanups.
I like it and will do it in JDK-8329628.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550378690
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 17:50:15 GMT, Stefan Karlsson wrote: >> No special rules here. I simply want to see all `virtual` methods explicitly >> and `override` is required by C++. >> I would like to keep it this way in these changes. I am investigating >> possibility to convert all these virtual methods to normal one to remove >> virtual table and virtual pointer (8 bytes) from CodeBlob class. > > `override` is not required by C++. You do however mark all virtual methods > with `override` if any of the functions are marked with `override`. I think > it would be good to have a HotSpot code style discussion about this (but not > in this RFE). I put `virtual/override` cleanup in CodeBlob as additional suggestion in followup RFE JDK-8329628. - PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550370826
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Mon, 1 Apr 2024 21:07:31 GMT, Vladimir Kozlov wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Removed not_used state of nmethod I filed RFEs: [JDK-8329628](https://bugs.openjdk.org/browse/JDK-8329628): Additional changes after JDK-8329332 [JDK-8329629](https://bugs.openjdk.org/browse/JDK-8329629): GC interfaces should work directly against nmethod instead of CodeBlob - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2035421134
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 16:00:01 GMT, Stefan Karlsson wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Removed not_used state of nmethod > > Nice! > > We've wanted to clean up some interfaces between the CodeCache and the GC > code by using nmethod closures instead of CodeBlob closures. This change (and > the Sweeper removal) makes it possible to do those cleanups. > > I've made a superficial pass over the patch to and left a few comments. Most > of those comments are things that would be nice to fix, but could also be > left as follow-up RFEs (if they are deemed to be worthy ideas to pursue). Thank you, @stefank, for great review. I addressed all your comments locally and with run testing in mach5 before pushing it. Except your suggestion about `find_blob_not_null()` - should be separate RFE. The same for suggestion "GC interfaces to work directly against nmethod instead of CodeBlob". - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2035354740
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 15:35:49 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed not_used state of nmethod
>
> src/hotspot/share/gc/x/xUnload.cpp line 78:
>
>> 76: class XIsUnloadingBehaviour : public IsUnloadingBehaviour {
>> 77: public:
>> 78: virtual bool has_dead_oop(nmethod* method) const {
>
> This now takes an `nmethod` argument, but still calls as_nmethod(). I think
> that should be removed from this, and all similar functions here in the GC
> code. If you want, I can do that as a follow-up RFE.
I decided to fix it in these changes.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550285238
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 15:49:00 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed not_used state of nmethod
>
> src/hotspot/share/runtime/frame.cpp line 208:
>
>> 206: address frame::raw_pc() const {
>> 207: if (is_deoptimized_frame()) {
>> 208: nmethod* nm = cb()->as_nmethod_or_null();
>
> Prexisting: It's weird that this code is using the `_or_null()` version when
> the code below does not null check the returned value.
Before [JDK-6921352](https://bugs.openjdk.org/browse/JDK-6921352) it was:
return ((nmethod*) cb())->deopt_handler_begin() - pc_return_offset;
I will add assert with check for null. We definitely expect here only nmethod.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550243895
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 16:38:13 GMT, Vladimir Kozlov wrote:
>> src/hotspot/share/code/codeCache.cpp line 1009:
>>
>>> 1007: int CodeCache::nmethod_count() {
>>> 1008: int count = 0;
>>> 1009: for (GrowableArrayIterator heap =
>>> _nmethod_heaps->begin(); heap != _nmethod_heaps->end(); ++heap) {
>>
>> Is there a reason why FOR_ALL_NMETHOD_HEAPS wasn't good fit here? I'm
>> wondering since the similar `CodeCache::blob_count()` still uses one of
>> these macros.
>
> No, `CodeCache::blob_count()` uses different macro `FOR_ALL_HEAPS(heap)`
> because it looks for all code blobs, not only nmethods.
>
> `CodeCache::nmethod_count()` is the only place where `FOR_ALL_NMETHOD_HEAPS `
> was used. So I decided to remove the macro.
I didn't say that blob_count used `FOR_ALL_NMETHODS_HEAP`. I wrote "one of
these macros". I still think this adds an inconsistency to the code that I
don't think is beneficial.
With that said, can't this be written as:
for (CodeHeap* heap : *_nmethod_heaps) {
Maybe yet another opportunity for cleanups.
>> src/hotspot/share/code/nmethod.cpp line 812:
>>
>>> 810: // By calling this nmethod entry barrier, it plays along and acts
>>> 811: // like any other nmethod found on the stack of a thread (fewer
>>> surprises).
>>> 812: nmethod* nm = as_nmethod_or_null();
>>
>> Calling as_nmethod_or_null() from within functions in the nmethod class is
>> suspicious. Shouldn't all such usages be removed? (I'm fine with doing that
>> as a separate change)
>
> Good catch! The code was moved from CompiledMethod where it made sense but
> now it is not needed. Here the change I will make:
>
> // like any other nmethod found on the stack of a thread (fewer
> surprises).
> -nmethod* nm = as_nmethod_or_null();
> -if (nm != nullptr && bs_nm->is_armed(nm)) {
> +nmethod* nm = this;
> +if (bs_nm->is_armed(nm)) {
>bool alive = bs_nm->nmethod_entry_barrier(nm);
Sounds good.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550216628
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550217712
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 16:29:03 GMT, Vladimir Kozlov wrote:
>> src/hotspot/share/code/codeBlob.hpp line 409:
>>
>>> 407:
>>> 408: // GC/Verification support
>>> 409: virtual void preserve_callee_argument_oops(frame fr, const
>>> RegisterMap *reg_map, OopClosure* f) override { /* nothing to do */ }
>>
>> In the GC code we usually have either virtual OR override, but not both.
>> Could we skip `virtual` here? Or does the compiler code usually use both?
>
> No special rules here. I simply want to see all `virtual` methods explicitly
> and `override` is required by C++.
> I would like to keep it this way in these changes. I am investigating
> possibility to convert all these virtual methods to normal one to remove
> virtual table and virtual pointer (8 bytes) from CodeBlob class.
`override` is not required by C++. You do however mark all virtual methods with
`override` if any of the functions are marked with `override`. I think it would
be good to have a HotSpot code style discussion about this (but not in this
RFE).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550206804
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 15:30:00 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed not_used state of nmethod
>
> src/hotspot/share/compiler/compileBroker.cpp line 1379:
>
>> 1377: if (osr_bci == InvocationEntryBci) {
>> 1378: // standard compilation
>> 1379: nmethod* method_code = method->code();
>
> Isn't the `method_code->is_nmethod()` redundant now?
An other good catch! It leads me to chase all redundant `is_nmethod()` and
`as_nmethod_*()` calls.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550187397
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 15:12:31 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed not_used state of nmethod
>
> src/hotspot/share/code/nmethod.cpp line 812:
>
>> 810: // By calling this nmethod entry barrier, it plays along and acts
>> 811: // like any other nmethod found on the stack of a thread (fewer
>> surprises).
>> 812: nmethod* nm = as_nmethod_or_null();
>
> Calling as_nmethod_or_null() from within functions in the nmethod class is
> suspicious. Shouldn't all such usages be removed? (I'm fine with doing that
> as a separate change)
Good catch! The code was moved from CompiledMethod where it made sense but now
it is not needed. Here the change I will make:
// like any other nmethod found on the stack of a thread (fewer surprises).
-nmethod* nm = as_nmethod_or_null();
-if (nm != nullptr && bs_nm->is_armed(nm)) {
+nmethod* nm = this;
+if (bs_nm->is_armed(nm)) {
bool alive = bs_nm->nmethod_entry_barrier(nm);
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550118967
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 15:01:22 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed not_used state of nmethod
>
> src/hotspot/share/code/codeCache.cpp line 1009:
>
>> 1007: int CodeCache::nmethod_count() {
>> 1008: int count = 0;
>> 1009: for (GrowableArrayIterator heap =
>> _nmethod_heaps->begin(); heap != _nmethod_heaps->end(); ++heap) {
>
> Is there a reason why FOR_ALL_NMETHOD_HEAPS wasn't good fit here? I'm
> wondering since the similar `CodeCache::blob_count()` still uses one of these
> macros.
No, `CodeCache::blob_count()` uses different macro `FOR_ALL_HEAPS(heap)`
because it looks for all code blobs, not only nmethods.
`CodeCache::nmethod_count()` is the only place where `FOR_ALL_NMETHOD_HEAPS `
was used. So I decided to remove the macro.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550087255
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 14:44:03 GMT, Stefan Karlsson wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed not_used state of nmethod
>
> src/hotspot/share/code/codeBlob.hpp line 409:
>
>> 407:
>> 408: // GC/Verification support
>> 409: virtual void preserve_callee_argument_oops(frame fr, const
>> RegisterMap *reg_map, OopClosure* f) override { /* nothing to do */ }
>
> In the GC code we usually have either virtual OR override, but not both.
> Could we skip `virtual` here? Or does the compiler code usually use both?
No special rules here. I simply want to see all `virtual` methods explicitly
and `override` is required by C++.
I would like to keep it this way in these changes. I am investigating
possibility to convert all these virtual methods to normal one to remove
virtual table and virtual pointer (8 bytes) from CodeBlob class.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550071713
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Wed, 3 Apr 2024 02:55:52 GMT, Vladimir Ivanov wrote: > What about `CompiledMethod_lock`? There's no `CompiledMethod` anymore, but > the lock name still refers to it. It was different changes [JDK-8226705](https://bugs.openjdk.org/browse/JDK-8226705). Renaming it will complicate these changes more than I wanted. I can do it in separate RFE. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2035031413
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Mon, 1 Apr 2024 21:07:31 GMT, Vladimir Kozlov wrote:
>> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE
>> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365)
>> which was used for AOT [JEP 295](https://openjdk.org/jeps/295)
>> implementation in JDK 9. The code was left in HotSpot assuming it will help
>> in a future. But during work on Leyden we decided to not use it. In Leyden
>> cached compiled code will be restored in CodeCache as normal nmethods: no
>> need to change VM's runtime and GC code to process them.
>>
>> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce
>> header size in separate changes. In these changes I did simple fields
>> reordering to keep small (1 byte) fields together.
>>
>> I do not see (and not expected) performance difference with these changes.
>>
>> Tested tier1-5, xcomp, stress. Running performance testing.
>>
>> I need help with testing on platforms which Oracle does not support.
>
> Vladimir Kozlov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed not_used state of nmethod
Nice!
We've wanted to clean up some interfaces between the CodeCache and the GC code
by using nmethod closures instead of CodeBlob closures. This change (and the
Sweeper removal) makes it possible to do those cleanups.
I've made a superficial pass over the patch to and left a few comments. Most of
those comments are things that would be nice to fix, but could also be left as
follow-up RFEs (if they are deemed to be worthy ideas to pursue).
src/hotspot/os/posix/signals_posix.cpp line 27:
> 25: #include "precompiled.hpp"
> 26: #include "code/codeCache.hpp"
> 27: #include "code/nmethod.hpp"
The include line needs to move down.
src/hotspot/share/code/codeBlob.hpp line 77:
> 75: // - data space
> 76:
> 77: enum CodeBlobKind : u1 {
It will probably be safer to change this to an enum class, so that the compiler
will help us if we mess up with the argument order when this is used in
function calls. I see that this patch switches the parameter order of some
functions, so I think it could be worth trying out.
src/hotspot/share/code/codeBlob.hpp line 409:
> 407:
> 408: // GC/Verification support
> 409: virtual void preserve_callee_argument_oops(frame fr, const RegisterMap
> *reg_map, OopClosure* f) override { /* nothing to do */ }
In the GC code we usually have either virtual OR override, but not both. Could
we skip `virtual` here? Or does the compiler code usually use both?
src/hotspot/share/code/codeBlob.hpp line 429:
> 427:SingletonBlob(
> 428: const char* name,
> 429: CodeBlobKind kind,
There's an alignment issue after this change.
src/hotspot/share/code/codeCache.cpp line 1009:
> 1007: int CodeCache::nmethod_count() {
> 1008: int count = 0;
> 1009: for (GrowableArrayIterator heap = _nmethod_heaps->begin();
> heap != _nmethod_heaps->end(); ++heap) {
Is there a reason why FOR_ALL_NMETHOD_HEAPS wasn't good fit here? I'm wondering
since the similar `CodeCache::blob_count()` still uses one of these macros.
src/hotspot/share/code/nmethod.cpp line 812:
> 810: // By calling this nmethod entry barrier, it plays along and acts
> 811: // like any other nmethod found on the stack of a thread (fewer
> surprises).
> 812: nmethod* nm = as_nmethod_or_null();
Calling as_nmethod_or_null() from within functions in the nmethod class is
suspicious. Shouldn't all such usages be removed? (I'm fine with doing that as
a separate change)
src/hotspot/share/code/nmethod.cpp line 1009:
> 1007: // Fill in default values for various flag fields
> 1008: void nmethod::init_defaults() {
> 1009: { // avoid uninitialized fields, even for short time periods
Should these curly braces be removed?
src/hotspot/share/code/nmethod.cpp line 2164:
> 2162: DTRACE_METHOD_UNLOAD_PROBE(method());
> 2163:
> 2164: // If a JVMTI agent has enabled the nmethod Unload event then
I think the event is still called CompiledMethodUnload, so this line should
probably be reverted.
src/hotspot/share/code/nmethod.hpp line 50:
> 48: class ScopeDesc;
> 49: class CompiledIC;
> 50: class MetadataClosure;
Maybe merge (and sort) this together with the other forward declarations?
src/hotspot/share/code/nmethod.hpp line 905:
> 903:
> 904: // printing support
> 905: void print() const override;
Here and a few other places you only use override and not also virtual. This is
inconsistent with other functions in this class. (FWIW, I prefer this style
with only the override qualifier).
src/hotspot/share/code/nmethod.inline.hpp line 60:
> 58: // (b) it is a deopt PC
> 59:
> 60: inline address nmethod::get_deopt_original_pc(const frame* fr) {
While reading this PR I wonder if this really belongs in the `nmethod` class or
if it would make more sense to have it as a member function in the `frame`
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Mon, 1 Apr 2024 21:07:31 GMT, Vladimir Kozlov wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Removed not_used state of nmethod Nice cleanup! Overall, looks very good. What about `CompiledMethod_lock`? There's no `CompiledMethod` anymore, but the lock name still refers to it. - Marked as reviewed by vlivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18554#pullrequestreview-1975392018
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
On Mon, 1 Apr 2024 21:07:31 GMT, Vladimir Kozlov wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Removed not_used state of nmethod I did not change `src/hotspot/share/code//codeHeapState.cpp` code which counts nmethods with `not_used` state by checking `(!nm->is_not_entrant()` after `(nm->is_in_use())`. Removing `not_used` does not affect it. The code is complicated and needs separate RFE if we decide to clean it. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2030560338
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]
> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE > [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) > which was used for AOT [JEP 295](https://openjdk.org/jeps/295) > implementation in JDK 9. The code was left in HotSpot assuming it will help > in a future. But during work on Leyden we decided to not use it. In Leyden > cached compiled code will be restored in CodeCache as normal nmethods: no > need to change VM's runtime and GC code to process them. > > I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce > header size in separate changes. In these changes I did simple fields > reordering to keep small (1 byte) fields together. > > I do not see (and not expected) performance difference with these changes. > > Tested tier1-5, xcomp, stress. Running performance testing. > > I need help with testing on platforms which Oracle does not support. Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Removed not_used state of nmethod - Changes: - all: https://git.openjdk.org/jdk/pull/18554/files - new: https://git.openjdk.org/jdk/pull/18554/files/7635b333..246ff68a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18554=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18554=00-01 Stats: 5 lines in 2 files changed: 0 ins; 3 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18554.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18554/head:pull/18554 PR: https://git.openjdk.org/jdk/pull/18554
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes
On Mon, 1 Apr 2024 18:15:43 GMT, Dean Long wrote: > The `not_used` state was introduced for AOT. It can go away now. Good catch, Dean. I want to keep `nmethod::make_not_used()` method because we use it in Leyden to keep AOT code (outside of CodeCache): [nmethod.hpp#L476](https://github.com/openjdk/leyden/blob/premain/src/hotspot/share/code/nmethod.hpp#L476) It does not use this flag value. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2030448462
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes
On Mon, 1 Apr 2024 00:19:32 GMT, Fei Yang wrote: >> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE >> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) >> which was used for AOT [JEP 295](https://openjdk.org/jeps/295) >> implementation in JDK 9. The code was left in HotSpot assuming it will help >> in a future. But during work on Leyden we decided to not use it. In Leyden >> cached compiled code will be restored in CodeCache as normal nmethods: no >> need to change VM's runtime and GC code to process them. >> >> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce >> header size in separate changes. In these changes I did simple fields >> reordering to keep small (1 byte) fields together. >> >> I do not see (and not expected) performance difference with these changes. >> >> Tested tier1-5, xcomp, stress. Running performance testing. >> >> I need help with testing on platforms which Oracle does not support. > > Hi, I also performed some tests (tier1-3 and hotspot:tier4) on linux-riscv64 > platform. Result looks good. @RealFYang and @offamitkumar thank you for testing. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2030425253
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes
On Fri, 29 Mar 2024 19:35:45 GMT, Vladimir Kozlov wrote: > Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE > [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) > which was used for AOT [JEP 295](https://openjdk.org/jeps/295) > implementation in JDK 9. The code was left in HotSpot assuming it will help > in a future. But during work on Leyden we decided to not use it. In Leyden > cached compiled code will be restored in CodeCache as normal nmethods: no > need to change VM's runtime and GC code to process them. > > I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce > header size in separate changes. In these changes I did simple fields > reordering to keep small (1 byte) fields together. > > I do not see (and not expected) performance difference with these changes. > > Tested tier1-5, xcomp, stress. Running performance testing. > > I need help with testing on platforms which Oracle does not support. The `not_used` state was introduced for AOT. It can go away now. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2030282409
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes
On Fri, 29 Mar 2024 19:35:45 GMT, Vladimir Kozlov wrote:
> Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE
> [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365)
> which was used for AOT [JEP 295](https://openjdk.org/jeps/295)
> implementation in JDK 9. The code was left in HotSpot assuming it will help
> in a future. But during work on Leyden we decided to not use it. In Leyden
> cached compiled code will be restored in CodeCache as normal nmethods: no
> need to change VM's runtime and GC code to process them.
>
> I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce
> header size in separate changes. In these changes I did simple fields
> reordering to keep small (1 byte) fields together.
>
> I do not see (and not expected) performance difference with these changes.
>
> Tested tier1-5, xcomp, stress. Running performance testing.
>
> I need help with testing on platforms which Oracle does not support.
I performed the build + testing `{fastdebug, release, slowdebug} X {tier1}` on
`s390x` and result looks fine.
-
PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2029655163
Re: RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes
On Fri, 29 Mar 2024 19:35:45 GMT, Vladimir Kozlov wrote: > Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE > [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) > which was used for AOT [JEP 295](https://openjdk.org/jeps/295) > implementation in JDK 9. The code was left in HotSpot assuming it will help > in a future. But during work on Leyden we decided to not use it. In Leyden > cached compiled code will be restored in CodeCache as normal nmethods: no > need to change VM's runtime and GC code to process them. > > I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce > header size in separate changes. In these changes I did simple fields > reordering to keep small (1 byte) fields together. > > I do not see (and not expected) performance difference with these changes. > > Tested tier1-5, xcomp, stress. Running performance testing. > > I need help with testing on platforms which Oracle does not support. Hi, I also performed some tests (tier1-3 and hotspot:tier4) on linux-riscv64 platform. Result looks good. - PR Comment: https://git.openjdk.org/jdk/pull/18554#issuecomment-2028963874
RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes
Revert [JDK-8152664](https://bugs.openjdk.org/browse/JDK-8152664) RFE [changes](https://github.com/openjdk/jdk/commit/b853eb7f5ca24eeeda18acbb14287f706499c365) which was used for AOT [JEP 295](https://openjdk.org/jeps/295) implementation in JDK 9. The code was left in HotSpot assuming it will help in a future. But during work on Leyden we decided to not use it. In Leyden cached compiled code will be restored in CodeCache as normal nmethods: no need to change VM's runtime and GC code to process them. I may work on optimizing `CodeBlob` and `nmethod` fields layout to reduce header size in separate changes. In these changes I did simple fields reordering to keep small (1 byte) fields together. I do not see (and not expected) performance difference with these changes. Tested tier1-5, xcomp, stress. Running performance testing. I need help with testing on platforms which Oracle does not support. - Commit messages: - remove trailing whitespace - 8329332: Remove CompiledMethod and CodeBlobLayout classes Changes: https://git.openjdk.org/jdk/pull/18554/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18554=00 Issue: https://bugs.openjdk.org/browse/JDK-8329332 Stats: 3885 lines in 118 files changed: 1281 ins; 1723 del; 881 mod Patch: https://git.openjdk.org/jdk/pull/18554.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18554/head:pull/18554 PR: https://git.openjdk.org/jdk/pull/18554
