Skip to content

Cache Client's Reflect Class Pointers at JITServer #20592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 18, 2024
Merged

Cache Client's Reflect Class Pointers at JITServer #20592

merged 2 commits into from
Nov 18, 2024

Conversation

luke-li-2003
Copy link
Contributor

@luke-li-2003 luke-li-2003 commented Nov 13, 2024 •
edited
Loading

Cache the reflect class pointer used by the client VM in its VMInfo for the JITServer so the JITServer can identify those classes.

Also ensures that the server will not make vm access requests in the vector API, and that it will not access the inside of any J9Class, but rather uses OpaqueClassBlock and requests information from the appropriate frontend.

@luke-li-2003
Copy link
Contributor Author

@mpirvu

mpirvu
mpirvu reviewed Nov 14, 2024
View reviewed changes
runtime/compiler/env/VMJ9.cpp
@@ -1307,31 +1307,64 @@ TR_J9VMBase::getReferenceElement(uintptr_t objectPointer, intptr_t elementIndex)
return (uintptr_t)J9JAVAARRAYOFOBJECT_LOAD(vmThread(), objectPointer, elementIndex);
}

TR_arrayTypeCode TR_J9VMBase::getPrimitiveArrayTypeCode(TR_OpaqueClassBlock* clazz)
TR_arrayTypeCode
TR_J9VMBase::getPrimitiveArrayTypeCode(TR_OpaqueClassBlock* clazz)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a frontend methods and we typically override the implementation in TR_J9ServerVM with the server specific code.

@mpirvu mpirvu self-assigned this Nov 14, 2024
@luke-li-2003 luke-li-2003 force-pushed the ServerCacheReflectClass branch 2 times, most recently from 7a5e301 to c3c9ada Compare November 14, 2024 21:20
@luke-li-2003 luke-li-2003 mentioned this pull request Nov 14, 2024
Closed
mpirvu
mpirvu approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

runtime/compiler/env/VMJ9Server.cpp
TR_arrayTypeCode
TR_J9ServerVM::getPrimitiveArrayTypeCode(TR_OpaqueClassBlock* clazz)
{
TR_ASSERT(isPrimitiveClass(clazz), "Expect primitive class in TR_J9VMBase::getPrimitiveArrayType");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the message to:
"Expect primitive class in TR_J9ServerVM::getPrimitiveArrayType"

runtime/compiler/env/VMJ9Server.hpp
@@ -265,6 +265,9 @@ class TR_J9ServerVM: public TR_J9VM
virtual bool isIndexableDataAddrPresent() override;
virtual bool isOffHeapAllocationEnabled() override;

virtual TR_arrayTypeCode getPrimitiveArrayTypeCode(TR_OpaqueClassBlock* clazz) override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's eliminate the extra spaces to make the code uniform

@mpirvu mpirvu marked this pull request as ready for review November 14, 2024 21:39
@mpirvu mpirvu requested a review from dsouzai as a code owner November 14, 2024 21:39
@mpirvu
Copy link
Contributor

mpirvu commented Nov 14, 2024

jenkins compile all java8

@mpirvu
Copy link
Contributor

mpirvu commented Nov 15, 2024

jenkins compile all jdk8

@mpirvu
Copy link
Contributor

mpirvu commented Nov 15, 2024

jenkins compile all jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Nov 15, 2024

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit,alinux64jit jdk23

@mpirvu
Copy link
Contributor

mpirvu commented Nov 15, 2024

Compilation error:

08:53:55  /home/jenkins/workspace/Build_JDK23_aarch64_linux_jit_Personal/openj9/runtime/compiler/optimizer/VectorAPIExpansion.cpp: In static member function 'static TR::Node* TR_VectorAPIExpansion::storeIntrinsicHandler(TR_VectorAPIExpansion*, TR::TreeTop*, TR::Node*, TR::DataType, TR::VectorLength, int32_t, TR_VectorAPIExpansion::handlerMode)':
08:53:55  /home/jenkins/workspace/Build_JDK23_aarch64_linux_jit_Personal/openj9/runtime/compiler/optimizer/VectorAPIExpansion.cpp:1692:96: error: 'valueToWrite' was not declared in this scope
08:53:55   1692 |    return transformStoreToArray(opt, treeTop, node, elementType, vectorLength, numLanes, mode, valueToWrite, base, offset, objType);
08:53:55        |                                                                                                ^~~~~~~~~~~~
08:53:56  [ 86%] Building CXX object runtime/gc_verbose_old_events/CMakeFiles/j9gcvrbevents_full.dir/VerboseEventLocalGCStart.cpp.o
08:53:56  runtime/compiler/CMakeFiles/j9jit.dir/build.make:1988: recipe for target 'runtime/compiler/CMakeFiles/j9jit.dir/optimizer/VectorAPIExpansion.cpp.o' failed

@luke-li-2003
Remove Server VM Access Request in VectorAPIExpansion.cpp
a97a4ba 
The server should make a specific request to the client in
getJ9ClassFromClassNode() and getVectorSizeFromVectorSpecies(),
instead of requesting a pointer from the KnownObjectTable as
that requires VM Access and will cause a fatal assertion fail
at J9KnownObjectTable.cpp:207.

Signed-off-by: Luke Li <[email protected]>
@luke-li-2003
Remove J9Class Occurrences in Vector API
326a800 
Substitute uses of J9Class in the vector API with TR_OpaqueClassBlock
with frontend references.

The Vector API, being a part of the optimizer, should never use the
inside of a J9Class

Signed-off-by: Luke Li <[email protected]>
@luke-li-2003 luke-li-2003 force-pushed the ServerCacheReflectClass branch from c3c9ada to 326a800 Compare November 15, 2024 22:34
@mpirvu
Copy link
Contributor

mpirvu commented Nov 15, 2024

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit,alinux64jit jdk23

@mpirvu
Copy link
Contributor

mpirvu commented Nov 16, 2024

All sanity tests passed, but openjdk ones have problems. The vector tests always fail. Apart from that I see the following:
On aarch64 jdk_lang_1 timeouts
On zlinux jdk_lang_1 and jdk_util_0 timeout
On xlinux jdk_util_0 timeouts

@mpirvu
Copy link
Contributor

mpirvu commented Nov 18, 2024

The timeouts mentioned in this PR have seen before, hence the code should be good to merge even though it does not solve all the issues JITServer has with Vector API.

@mpirvu mpirvu merged commit 77f153d into eclipse-openj9:master Nov 18, 2024
10 of 15 checks passed
@pshipton pshipton added the comp:jitserver Artifacts related to JIT-as-a-Service project label Nov 18, 2024
@pshipton pshipton mentioned this pull request Nov 18, 2024
Closed
h3110n3rv3 added a commit to h3110n3rv3/openj9 that referenced this pull request Nov 19, 2024
@h3110n3rv3
Squashed commit of the following:
09434fe 
commit 44766de
Merge: 975e741 fe89ec5
Author: Marius Pirvu <[email protected]>
Date:   Tue Nov 19 09:18:48 2024 -0500

    Merge pull request eclipse-openj9#20614 from JamesKingdon/trampolineResetFix

    Reset trampoline pointers to top of space

commit 975e741
Merge: 9618d90 9f566fa
Author: Peter Shipton <[email protected]>
Date:   Tue Nov 19 08:34:48 2024 -0500

    Merge pull request eclipse-openj9#20631 from tajila/jfr3

    Disable JFR JCL APIs

commit 9618d90
Merge: 8f240e6 36dc396
Author: Dmitri Pivkine <[email protected]>
Date:   Mon Nov 18 21:45:37 2024 -0500

    Merge pull request eclipse-openj9#20630 from amicic/VLHGC_barrier_32bit

    Don't use Offheap APIs for 32 bit

commit 9f566fa
Author: tajila <[email protected]>
Date:   Mon Nov 18 20:05:47 2024 -0500

    Disable JFR JCL APIs

    Disable JFR JCL APIs until JFR natives are implemented.

    Related to: eclipse-openj9#20607

    Signed-off-by: tajila <[email protected]>

commit 8f240e6
Merge: 7534b4e 6662a08
Author: Keith W. Campbell <[email protected]>
Date:   Mon Nov 18 20:05:46 2024 -0500

    Merge pull request eclipse-openj9#20621 from tajila/jfr2

    Fix conditon in JFR buffer test

commit 6662a08
Author: tajila <[email protected]>
Date:   Mon Nov 18 15:49:59 2024 -0500

    Fix conditon in JFR buffer test

    Signed-off-by: tajila <[email protected]>

commit 36dc396
Author: Aleksandar Micic <[email protected]>
Date:   Mon Nov 18 19:11:09 2024 -0500

    Don't use Offheap APIs for 32 bit

    A couple of Offheap APIs are guarded with 64 bit compile flag.

    A more accurate fix would be to guard them with Offheap specific build
    flag (and do not compile the whole dir for 32bit), but it would require
    more complex changes.

    Signed-off-by: Aleksandar Micic <[email protected]>

commit 7534b4e
Merge: 3d719d4 276f38f
Author: Graham Chapman <[email protected]>
Date:   Mon Nov 18 18:14:00 2024 -0500

    Merge pull request eclipse-openj9#20620 from tajila/jfr3

    Release jfrSampler lock before acquiring VMaccess

commit 3d719d4
Merge: 247d6ec c0ff1fc
Author: Peter Shipton <[email protected]>
Date:   Mon Nov 18 17:17:42 2024 -0500

    Merge pull request eclipse-openj9#20622 from AdamBrousseau/cuda_docker_non_dockerhub

    Pull cuda image from nvcr

commit 247d6ec
Merge: 068838e fee4aeb
Author: Aleksandar Micic <[email protected]>
Date:   Mon Nov 18 16:55:50 2024 -0500

    Merge pull request eclipse-openj9#20514 from LinHu2016/coding-std-update2

    Clean up MethodTypesIterator class

commit 068838e
Merge: d98d498 3dc8eb4
Author: Aleksandar Micic <[email protected]>
Date:   Mon Nov 18 16:46:15 2024 -0500

    Merge pull request eclipse-openj9#20564 from LinHu2016/off-heap-incremental4

    New vm/gc API j9gc_objaccess_indexableDataDisplacement

commit c0ff1fc
Author: Adam Brousseau <[email protected]>
Date:   Mon Nov 18 16:03:35 2024 -0500

    Pull cuda image from nvcr

    In the event we are rate-limited or restricted from pulling
    from DockerHub, this should work around the problem by pulling
    from Nvidia's site.

    Issue runtimes/automation/122

    Signed-off-by: Adam Brousseau <[email protected]>

commit 3dc8eb4
Author: lhu <[email protected]>
Date:   Fri Nov 8 11:50:32 2024 -0500

    New vm/gc API j9gc_objaccess_indexableDataDisplacement

    new API j9gc_objaccess_indexableDataDisplacement used by the JIT.
    Returns the displacement for the data of moved array object.
    For adjacent Array and 0 size array, displacement = dst - src
    For Off-heap Array, displacement = 0
    should only be called for off-heap eanbled case.

    Signed-off-by: lhu <[email protected]>

commit d98d498
Merge: 77f153d ee35ed5
Author: Graham Chapman <[email protected]>
Date:   Mon Nov 18 14:52:20 2024 -0500

    Merge pull request eclipse-openj9#20611 from tajila/jfr2

    Prevent writes to free'd JFR buffer

commit 77f153d
Merge: 038df36 326a800
Author: Marius Pirvu <[email protected]>
Date:   Mon Nov 18 14:09:15 2024 -0500

    Merge pull request eclipse-openj9#20592 from luke-li-2003/ServerCacheReflectClass

    Cache Client's Reflect Class Pointers at JITServer

commit 276f38f
Author: tajila <[email protected]>
Date:   Mon Nov 18 13:59:52 2024 -0500

    Release jfrSampler lock before acquiring VMaccess

    Signed-off-by: tajila <[email protected]>

commit ee35ed5
Author: tajila <[email protected]>
Date:   Sun Nov 17 16:06:08 2024 -0500

    Prevent writes to free'd JFR buffer

    Also, check that threadObject is not NULL before dereference.

    Signed-off-by: tajila <[email protected]>

commit 038df36
Merge: 224374d 0573107
Author: Aleksandar Micic <[email protected]>
Date:   Mon Nov 18 11:12:43 2024 -0500

    Merge pull request eclipse-openj9#20596 from LinHu2016/off-heap-incremental6

    Update verbose GC for off-heap

commit 224374d
Merge: 6289c28 d56338f
Author: Babneet Singh <[email protected]>
Date:   Mon Nov 18 10:20:28 2024 -0500

    Merge pull request eclipse-openj9#20606 from ThanHenderson/omr-unnamed

    Fix OMR_GLUE unnamed spelling mistake

commit 6289c28
Merge: efaa4f6 1af5097
Author: Tobi <[email protected]>
Date:   Mon Nov 18 09:56:54 2024 -0500

    Merge pull request eclipse-openj9#20605 from pshipton/cracbean

    Move CRaCMXBeanImpl and export jdk.crac.management

commit 1af5097
Author: Peter Shipton <[email protected]>
Date:   Fri Nov 15 14:28:45 2024 -0500

    Move CRaCMXBeanImpl and export jdk.crac.management

    Issue eclipse-openj9#20587

    Signed-off-by: Peter Shipton <[email protected]>

commit 326a800
Author: Luke Li <[email protected]>
Date:   Thu Nov 14 16:13:54 2024 -0500

    Remove J9Class Occurrences in Vector API

    Substitute uses of J9Class in the vector API with TR_OpaqueClassBlock
    with frontend references.

    The Vector API, being a part of the optimizer, should never use the
    inside of a J9Class

    Signed-off-by: Luke Li <[email protected]>

commit a97a4ba
Author: Luke Li <[email protected]>
Date:   Mon Oct 28 09:31:53 2024 -0400

    Remove Server VM Access Request in VectorAPIExpansion.cpp

    The server should make a specific request to the client in
    getJ9ClassFromClassNode() and getVectorSizeFromVectorSpecies(),
    instead of requesting a pointer from the KnownObjectTable as
    that requires VM Access and will cause a fatal assertion fail
    at J9KnownObjectTable.cpp:207.

    Signed-off-by: Luke Li <[email protected]>

commit fe89ec5
Author: James Kingdon <[email protected]>
Date:   Fri Nov 15 15:24:35 2024 -0500

    Reset trampoline pointers to top of space

    After flushing the code cache under FSD there is a call to resetTrampolines.
    The _trampolineAllocationMark and _trampolineReservationMark need to be reset
    to their starting positions but were incorrectly set to the end of the trampoline
    space. As a result all active segments at the time of the cache flush are marked
    as full (assuming a platform that uses trampolines). This can be seen by taking
    a JIT verbose log with option `codecache` which will show

    CODECACHE:  CodeCache 00007E759000C660 marked as full in reserveSpaceForTrampoline

commit d56338f
Author: Nathan Henderson <[email protected]>
Date:   Fri Nov 15 11:32:46 2024 -0800

    Fix OMR_GLUE unnamed spelling mistake

    Signed-off-by: Nathan Henderson <[email protected]>

commit 0573107
Author: lhu <[email protected]>
Date:   Tue Nov 12 12:09:29 2024 -0500

    Update verbose GC for off-heap

     -new outputOffHeapInfo in CopyForward and Mark for balancedGC
      <offheap candidates="xxx" cleared="xxx" />"
     - output offheap-objects in MemoryInfo
      <offheap-objects objects="xxx" bytes="xxx" />

    Signed-off-by: lhu <[email protected]>

commit fee4aeb
Author: lhu <[email protected]>
Date:   Tue Nov 5 10:51:09 2024 -0500

    Clean up MethodTypesIterator class

    Use consistent data types and code formatting

    Signed-off-by: lhu <[email protected]>
@mpirvu mpirvu mentioned this pull request Dec 10, 2024
Merged
@luke-li-2003 luke-li-2003 mentioned this pull request Jan 30, 2025
Closed
@luke-li-2003 luke-li-2003 deleted the ServerCacheReflectClass branch February 7, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpirvu mpirvu mpirvu approved these changes

@dsouzai dsouzai Awaiting requested review from dsouzai dsouzai is a code owner

Assignees

@mpirvu mpirvu

Labels
comp:jitserver Artifacts related to JIT-as-a-Service project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@luke-li-2003 @mpirvu @pshipton