On Thu, 4 Apr 2024 00:05:20 GMT, Vladimir Kozlov <k...@openjdk.org> 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
