Skip to content

Commit 03efb95

Browse files
committed
Use comp->permanentLoaders() for client retained methods analysis
This was avoiding getting the permanent loaders from the compilation object to guard against a scenario where the server side of the compilation was aware of more permanent loaders than the client side. That scenario is no longer possible because a compilation on the server now limits itself to the same set of permanent loaders that the corresponding compilation is aware of on the client. It's no longer necessary for J9::RetainedMethodSet to allow the caller to specify the permanent loaders on creation.
1 parent ff25310 commit 03efb95

File tree

2 files changed

+9
-55
lines changed

2 files changed

+9
-55
lines changed

runtime/compiler/env/J9RetainedMethodSet.cpp

Lines changed: 7 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,12 @@ J9::RetainedMethodSet *
152152
J9::RetainedMethodSet::create(
153153
TR::Compilation *comp,
154154
TR_ResolvedMethod *method,
155-
const TR::vector<J9::ResolvedInlinedCallSite, TR::Region&> &inliningTable,
156-
const TR::vector<J9ClassLoader*, TR::Region&> *permanentLoaders)
155+
const TR::vector<J9::ResolvedInlinedCallSite, TR::Region&> &inliningTable)
157156
{
158157
TR::Region &region = comp->trMemory()->heapMemoryRegion();
159158
auto *inlTab = new (region) VectorInliningTable(comp, inliningTable);
160159
auto *result = new (region) J9::RetainedMethodSet(comp, method, NULL, inlTab);
161-
result->init(permanentLoaders);
160+
result->init();
162161
return result;
163162
}
164163

@@ -205,20 +204,15 @@ traceNamedLoader(
205204
}
206205

207206
void
208-
J9::RetainedMethodSet::init(
209-
const TR::vector<J9ClassLoader*, TR::Region&> *permanentLoaders)
207+
J9::RetainedMethodSet::init()
210208
{
211209
if (parent() == NULL)
212210
{
213211
bool trace = comp()->getOption(TR_TraceRetainedMethods);
214212

215-
if (permanentLoaders == NULL)
216-
{
217-
permanentLoaders = &comp()->permanentLoaders();
218-
}
219-
220-
auto end = permanentLoaders->end();
221-
for (auto it = permanentLoaders->begin(); it != end; it++)
213+
auto permanentLoaders = comp()->permanentLoaders();
214+
auto end = permanentLoaders.end();
215+
for (auto it = permanentLoaders.begin(); it != end; it++)
222216
{
223217
J9ClassLoader *loader = *it;
224218
_loaders.insert(loader);
@@ -792,47 +786,8 @@ J9::RepeatRetainedMethodsAnalysis::analyzeOnClient(
792786
return result;
793787
}
794788

795-
// Ensure that the set of permanent loaders used for analysis is up to date.
796-
// Otherwise, we'd use the set belonging to the compilation object, and the
797-
// following sequence of events would be possible:
798-
//
799-
// 1. Permanent loaders are determined for this compilation on the client
800-
// before sending the compilation request.
801-
// 2. The client JIT is informed of a newly discovered permanent loader.
802-
// 3. A different compilation thread sends another compilation request that
803-
// informs the server of the new permanent loader.
804-
// 4. The server takes note of the new permanent loader.
805-
// 5. Permanent loaders are determined for this compilation on the server.
806-
//
807-
// In this case, this compilation's permanent loaders on the client would be
808-
// a strict subset of those on the server, but (if the sets aren't identical)
809-
// the opposite containment relationship is required.
810-
//
811-
// By freshly collecting the permanent loaders here, we guarantee that the
812-
// server's set will be a (possibly non-strict) subset of the client's. To
813-
// see why, consider any permanent loader L known to the server in the
814-
// current compilation. We know that the following events must happen in the
815-
// order in which they are listed, since each causally precedes the next:
816-
//
817-
// - L is found to be permanent on the client and reported to the JIT.
818-
// - L is sent to the server.
819-
// - L is considered permanent for the current compilation on the server.
820-
// - The current compilation does getDataForClient() on the server.
821-
// - The current compilation starts analyzeOnClient() on the client (in progress).
822-
// - The client freshly determines the permanent loaders (just below).
823-
//
824-
// Therefore, L is included in the freshly determined permanent loaders.
825-
//
826-
TR::PersistentInfo *persistentInfo = comp->getPersistentInfo();
827-
TR_PersistentClassLoaderTable *loaderTable =
828-
persistentInfo->getPersistentClassLoaderTable();
829-
830-
TR::Region &stackRegion = comp->trMemory()->currentStackRegion();
831-
TR::vector<J9ClassLoader*, TR::Region&> permanentLoaders(stackRegion);
832-
loaderTable->getPermanentLoaders(comp->fej9()->vmThread(), permanentLoaders);
833-
834789
J9::RetainedMethodSet *root = J9::RetainedMethodSet::create(
835-
comp, comp->getMethodBeingCompiled(), inliningTable, &permanentLoaders);
790+
comp, comp->getMethodBeingCompiled(), inliningTable);
836791

837792
uint32_t n = (uint32_t)inliningTable.size();
838793
TR::vector<J9::RetainedMethodSet*, TR::Region&> retainedMethods(comp->region());

runtime/compiler/env/J9RetainedMethodSet.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ class RetainedMethodSet : public OMR::RetainedMethodSet
6464
static J9::RetainedMethodSet *create(
6565
TR::Compilation *comp,
6666
TR_ResolvedMethod *method,
67-
const TR::vector<J9::ResolvedInlinedCallSite, TR::Region&> &inliningTable,
68-
const TR::vector<J9ClassLoader*, TR::Region&> *permanentLoaders = NULL);
67+
const TR::vector<J9::ResolvedInlinedCallSite, TR::Region&> &inliningTable);
6968

7069
static const TR::vector<J9::ResolvedInlinedCallSite, TR::Region&> &
7170
copyInliningTable(TR::Compilation *comp, J9JITExceptionTable *metadata);
@@ -116,7 +115,7 @@ class RetainedMethodSet : public OMR::RetainedMethodSet
116115

117116
private:
118117

119-
void init(const TR::vector<J9ClassLoader*, TR::Region&> *permanentLoaders = NULL);
118+
void init();
120119
void attestWillRemainLoaded(TR_ResolvedMethod *method);
121120
void scan(J9Class *clazz);
122121
void scan(J9ClassLoader *loader);

0 commit comments

Comments
 (0)