Skip to content
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.

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

Prevent JIT bodies from strictly outliving methods inlined into them #21216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions runtime/compiler/build/files/common.mk.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ JIT_PRODUCT_BACKEND_SOURCES+=\
omr/compiler/env/OMRDebugEnv.cpp \
omr/compiler/env/OMRObjectModel.cpp \
omr/compiler/env/OMRPersistentInfo.cpp \
omr/compiler/env/OMRRetainedMethodSet.cpp \
omr/compiler/env/OMRVMEnv.cpp \
omr/compiler/env/Region.cpp \
omr/compiler/env/SegmentAllocator.cpp \
Expand Down Expand Up @@ -306,6 +307,7 @@ JIT_PRODUCT_SOURCE_FILES+=\
compiler/env/J9KnownObjectTable.cpp \
compiler/env/J9ObjectModel.cpp \
compiler/env/J9PersistentInfo.cpp \
compiler/env/J9RetainedMethodSet.cpp \
compiler/env/J9SegmentAllocator.cpp \
compiler/env/J9SegmentCache.cpp \
compiler/env/J9SegmentProvider.cpp \
Expand Down
5 changes: 4 additions & 1 deletion runtime/compiler/codegen/J9CodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include "env/VMJ9.h"
#include "env/jittypes.h"
#include "env/j9method.h"
#include "env/OMRRetainedMethodSet.hpp"
#include "il/AutomaticSymbol.hpp"
#include "il/Block.hpp"
#include "il/LabelSymbol.hpp"
Expand Down Expand Up @@ -4814,11 +4815,13 @@ bool
J9::CodeGenerator::mustGenerateSwitchToInterpreterPrePrologue()
{
TR::Compilation *comp = self()->comp();
TR_ResolvedMethod *bondMethod = NULL;

return comp->usesPreexistence() ||
comp->getOption(TR_EnableHCR) ||
!comp->fej9()->isAsyncCompilation() ||
comp->getOption(TR_FullSpeedDebug);
comp->getOption(TR_FullSpeedDebug) ||
comp->retainedMethods()->bondMethods().next(&bondMethod);
}

extern void VMgenerateCatchBlockBBStartPrologue(TR::Node *node, TR::Instruction *fenceInstruction, TR::CodeGenerator *cg);
Expand Down
59 changes: 59 additions & 0 deletions runtime/compiler/compile/J9Compilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "control/Recompilation.hpp"
#include "control/RecompilationInfo.hpp"
#include "env/j9method.h"
#include "env/J9RetainedMethodSet.hpp"
#include "env/TRMemory.hpp"
#include "env/VMJ9.h"
#include "env/VMAccessCriticalSection.hpp"
Expand Down Expand Up @@ -185,6 +186,7 @@ J9::Compilation::Compilation(int32_t id,
_j9VMThread(j9vmThread),
_monitorAutos(m),
_monitorAutoSymRefsInCompiledMethod(getTypedAllocator<TR::SymbolReference*>(self()->allocator())),
_keepaliveClasses(heapMemoryRegion),
_classForOSRRedefinition(m),
_classForStaticFinalFieldModification(m),
_profileInfo(NULL),
Expand Down Expand Up @@ -229,6 +231,10 @@ J9::Compilation::Compilation(int32_t id,
for (int i = 0; i < CACHED_CLASS_POINTER_COUNT; i++)
_cachedClassPointers[i] = NULL;

if (!self()->ilGenRequest().details().supportsInvalidation())
{
self()->getOptions()->setOption(TR_DontInlineUnloadableMethods);
}

// Add known object index to parm 0 so that other optmizations can be unlocked.
// It is safe to do so because method and method symbols of a archetype specimen
Expand Down Expand Up @@ -1671,6 +1677,59 @@ J9::Compilation::populateAOTMethodDependencies(TR_OpaqueClassBlock *definingClas
}
#endif /* !defined(PERSISTENT_COLLECTIONS_UNSUPPORTED) */

OMR::RetainedMethodSet *
J9::Compilation::createRetainedMethods(TR_ResolvedMethod *method)
{
#if defined(J9VM_OPT_JITSERVER)
if (self()->isRemoteCompilation())
{
TR_ASSERT_FATAL(false, "client must not use Compilation::retainedMethods()");
}
#endif

if (mustTrackRetainedMethods())
{
return J9::RetainedMethodSet::create(self(), method);
}
else
{
return OMR::Compilation::createRetainedMethods(method);
}
}

bool
J9::Compilation::mustTrackRetainedMethods()
{
if (self()->compileRelocatableCode())
{
// AOT: Relationships between class loaders seen at compile time won't
// necessarily still hold at load time, so there's no point in tracking
// retained method sets. At load, the inlining table will be available,
// and bonds will be created as needed.
return false;
}

if (TR::Options::getCmdLineOptions()->getOption(TR_NoClassGC))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the options object attached to this compilation? This is propagated to JITServer.

{
return false;
}

return !self()->getOption(TR_AllowJitBodyToOutliveInlinedCode)
|| self()->getOption(TR_DontInlineUnloadableMethods);
}

void
J9::Compilation::addKeepaliveClass(TR_OpaqueClassBlock *c)
{
_keepaliveClasses.insert(c);
if (self()->getOption(TR_TraceRetainedMethods))
{
int32_t len;
const char *name = TR::Compiler->cls.classNameChars(self(), c, len);
traceMsg(self(), "Added global keepalive class %p %.*s\n", c, len, name);
}
}

#if defined(J9VM_OPT_JITSERVER)
void
J9::Compilation::addSerializationRecord(const AOTCacheRecord *record, uintptr_t reloDataOffset)
Expand Down
48 changes: 47 additions & 1 deletion runtime/compiler/compile/J9Compilation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,15 @@ namespace J9 { typedef J9::Compilation CompilationConnector; }
#include "compile/CompilationTypes.hpp"
#include "control/Options.hpp"
#include "control/Options_inlines.hpp"
#include "infra/set.hpp"
#include "infra/Statistics.hpp"
#include "env/CompilerEnv.hpp"
#include "env/OMRMemory.hpp"
#include "compile/AOTClassInfo.hpp"
#include "runtime/SymbolValidationManager.hpp"
#include "env/PersistentCollections.hpp"


namespace J9 { class RetainedMethodSet; }
class TR_AOTGuardSite;
class TR_FrontEnd;
class TR_ResolvedMethod;
Expand Down Expand Up @@ -396,6 +397,49 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector

TR::SymbolValidationManager *getSymbolValidationManager() { return _symbolValidationManager; }

// overrides OMR::Compilation::createRetainedMethods(TR_ResolvedMethod*)
OMR::RetainedMethodSet *createRetainedMethods(TR_ResolvedMethod *method);

/**
* \brief Determine whether retained methods need to be tracked.
*
* If they do, then J9::RetainedMethodSet will be used. Otherwise, the base
* OMR::RetainedMethodSet will be used instead, which does no tracking.
*
* \return true if tracking is needed, false otherwise
*/
bool mustTrackRetainedMethods();

/**
* \brief Get the set of classes to keep alive.
*
* During optimization, these are classes that should be kept alive due to
* IL transformations, as opposed to the keepalives in retainedMethods(),
* which are due to inlining. They are kept separately because the API of
* OMR::RetainedMethodSet is designed to avoid assuming that unloading
* proceeds at any granularity coarser than per-method.
*
* \return the set of classes to keep alive
*/
const TR::set<TR_OpaqueClassBlock*> &keepaliveClasses() { return _keepaliveClasses; }

/**
* \brief Add a keepalive class.
*
* This is only for cases where an IL transformation based on known objects
* causes the IL to directly use a class (i.e. with loadaddr) or one of
* its members, when it didn't previously. After such a transformation, the
* known objects could end up being unused, in which case they wouldn't
* guarantee on their own that the class remains loaded at the point of use.
*
* If the IL is modified to use a member that will necessarily remain loaded
* at the point of use anyway, e.g. an instance method, then no keepalive is
* needed.
*
* \param c the class to keep alive
*/
void addKeepaliveClass(TR_OpaqueClassBlock *c);

/**
* \brief Determine whether it's currently expected to be possible to add
* OSR assumptions and corresponding fear points somewhere in the method.
Expand Down Expand Up @@ -518,6 +562,8 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector
TR_Array<List<TR::RegisterMappedSymbol> *> _monitorAutos;
TR::list<TR::SymbolReference*> _monitorAutoSymRefsInCompiledMethod;

TR::set<TR_OpaqueClassBlock*> _keepaliveClasses;

TR_Array<TR_OpaqueClassBlock*> _classForOSRRedefinition;
// Classes that have their static final fields folded and need assumptions
TR_Array<TR_OpaqueClassBlock*> _classForStaticFinalFieldModification;
Expand Down
9 changes: 9 additions & 0 deletions runtime/compiler/control/CompilationThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8877,6 +8877,15 @@ TR::CompilationInfoPerThreadBase::wrappedCompile(J9PortLibrary *portLib, void *
methodInfo->incrementNumberOfInlinedMethodRedefinition();
if (methodInfo->getNumberOfInlinedMethodRedefinition() >= 2)
options->setOption(TR_DisableNextGenHCR);

// If this method has been invalidated due to unloading of
// inlined code, then from now on we should only inline
// methods that will not be unloaded before this one.
auto invReasons = methodInfo->invalidationReasons();
if (invReasons.contains(TR_JitBodyInvalidations::Unloading))
{
options->setOption(TR_DontInlineUnloadableMethods);
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion runtime/compiler/control/J9Recompilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "compile/Compilation.hpp"
#include "control/Options.hpp"
#include "compile/SymbolReferenceTable.hpp"
#include "env/OMRRetainedMethodSet.hpp"
#include "env/VMJ9.h"
#include "env/VerboseLog.hpp"
#include "runtime/J9Profiler.hpp"
Expand Down Expand Up @@ -456,10 +457,12 @@ J9::Recompilation::createProfilers()
bool
J9::Recompilation::couldBeCompiledAgain()
{
TR_ResolvedMethod *bondMethod = NULL;
return
self()->shouldBeCompiledAgain() ||
_compilation->usesPreexistence() ||
_compilation->getOption(TR_EnableHCR);
_compilation->getOption(TR_EnableHCR) ||
_compilation->retainedMethods()->bondMethods().next(&bondMethod);
}


Expand Down
47 changes: 47 additions & 0 deletions runtime/compiler/control/JITClientCompilationThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "env/j9methodServer.hpp"
#include "env/JITServerPersistentCHTable.hpp"
#include "env/JSR292Methods.h"
#include "env/J9RetainedMethodSet.hpp"
#include "env/TypeLayout.hpp"
#include "env/ut_j9jit.h"
#include "env/VerboseLog.hpp"
Expand Down Expand Up @@ -3013,6 +3014,52 @@ handleServerMessage(JITServer::ClientStream *client, TR_J9VM *fe, JITServer::Mes
client->write(response, classInfos);
}
break;
case MessageType::RetainedMethodSet_createMirror:
{
auto recv = client->getRecvData<void*, void*>();
auto parent = (J9::RetainedMethodSet*)std::get<0>(recv);
auto method = (TR_ResolvedJ9Method*)std::get<1>(recv);
J9::RetainedMethodSet *mirror = NULL;
J9::RetainedMethodSet::ScanLog scanLog;
if (parent == NULL)
{
mirror = J9::RetainedMethodSet::create(comp, method, &scanLog);
}
else
{
mirror = parent->createChild(method, &scanLog);
}

client->write(
response, mirror, scanLog._addedAnonClass, scanLog._addedLoaders);
}
break;
case MessageType::RetainedMethodSet_scan:
{
auto recv = client->getRecvData<void*, J9Class*>();
auto mirror = (J9::RetainedMethodSet*)std::get<0>(recv);
auto clazz = std::get<1>(recv);
J9::RetainedMethodSet::ScanLog scanLog;
mirror->scanForClient(clazz, &scanLog);
client->write(response, scanLog._addedAnonClass, scanLog._addedLoaders);
}
break;
case MessageType::RetainedMethodSet_keepalive:
{
auto recv = client->getRecvData<void*>();
auto mirror = (J9::RetainedMethodSet*)std::get<0>(recv);
mirror->keepalive();
client->write(response, JITServer::Void());
}
break;
case MessageType::RetainedMethodSet_bond:
{
auto recv = client->getRecvData<void*>();
auto mirror = (J9::RetainedMethodSet*)std::get<0>(recv);
mirror->bond();
client->write(response, JITServer::Void());
}
break;
default:
// It is vital that this remains a hard error during dev!
TR_ASSERT(false, "JITServer: handleServerMessage received an unknown message type: %d\n", response);
Expand Down
1 change: 1 addition & 0 deletions runtime/compiler/control/RecompilationInfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class TR_JitBodyInvalidations
HCR, // outermost method has been obsoleted by HCR
Preexistence, // preexistence assumption has been invalidated
PostRestoreExclude, // method is excluded post-restore, should be interpreted
Unloading, // an inlined method has been unloaded
};

bool isEmpty() const { return _flags.getValue() == 0; }
Expand Down
Loading