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

Move JFR loadedClassCount to J9JavaVM #20987

Merged
merged 1 commit into from
Jan 22, 2025
Merged
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
6 changes: 3 additions & 3 deletions runtime/oti/j9nonbuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -3632,9 +3632,6 @@ typedef struct J9ClassLoader {
omrthread_rwmutex_t cpEntriesMutex;
UDATA initClassPathEntryCount;
UDATA asyncGetCallTraceUsed;
#if defined(J9VM_OPT_JFR)
UDATA loadedClassCount;
#endif /* defined(J9VM_OPT_JFR) */
} J9ClassLoader;

#define J9CLASSLOADER_SHARED_CLASSES_ENABLED 8
Expand Down Expand Up @@ -6307,6 +6304,9 @@ typedef struct J9JavaVM {
VMSnapshotImplPortLibrary *vmSnapshotImplPortLibrary;
const char *vmSnapshotFilePath;
#endif /* defined(J9VM_OPT_SNAPSHOTS) */
#if defined(J9VM_OPT_JFR)
UDATA loadedClassCount;
#endif /* defined(J9VM_OPT_JFR) */
} J9JavaVM;

#define J9JFR_SAMPLER_STATE_UNINITIALIZED 0
Expand Down
4 changes: 0 additions & 4 deletions runtime/vm/classallocation.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,6 @@ allocateClassLoader(J9JavaVM *javaVM)
classLoader->packageHashTable = hashPackageTableNew(javaVM, INITIAL_PACKAGE_HASHTABLE_SIZE);
#endif /* JAVA_SPEC_VERSION > 8 */

#if defined(J9VM_OPT_JFR)
classLoader->loadedClassCount = 0;
#endif /* defined(J9VM_OPT_JFR) */

/* Allocate classLocationHashTable only for bootloader which is the first classloader to be allocated.
* The classLoader being allocated must be the bootloader if javaVM->systemClassLoader is NULL.
*/
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/createramclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2169,7 +2169,7 @@ internalCreateRAMClassDone(J9VMThread *vmThread, J9ClassLoader *classLoader, J9C
}

#if defined(J9VM_OPT_JFR)
hostClassLoader->loadedClassCount += 1;
javaVM->loadedClassCount += 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be done atomically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The calling thread should be holding the classtable mutex so we don't need atomic?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems you're right, however, it would help if this function called out that expectation (like line 759 does for computeVTable().

#endif /* defined(J9VM_OPT_JFR) */

/* Create all the method IDs if class load is hooked */
Expand Down
16 changes: 1 addition & 15 deletions runtime/vm/jfr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,6 @@ void
jfrClassLoadingStatistics(J9VMThread *currentThread)
{
J9JavaVM *vm = currentThread->javaVM;
J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions;
J9JFRClassLoadingStatistics *jfrEvent = (J9JFRClassLoadingStatistics *)reserveBuffer(currentThread, sizeof(J9JFRClassLoadingStatistics));

if (NULL != jfrEvent) {
Expand All @@ -1030,20 +1029,7 @@ jfrClassLoadingStatistics(J9VMThread *currentThread)
UDATA unloadedAnonClassCount = 0;
vm->memoryManagerFunctions->j9gc_get_cumulative_class_unloading_stats(currentThread, &unloadedAnonClassCount, &unloadedClassCount, NULL);
jfrEvent->unloadedClassCount = (I_64)(unloadedClassCount + unloadedAnonClassCount);

internalReleaseVMAccess(currentThread);

J9ClassLoaderWalkState walkState = {0};
J9ClassLoader *classLoader = vmFuncs->allClassLoadersStartDo(&walkState, vm, 0);

while (NULL != classLoader) {
jfrEvent->loadedClassCount += classLoader->loadedClassCount;

classLoader= vmFuncs->allClassLoadersNextDo(&walkState);
}
vmFuncs->allClassLoadersEndDo(&walkState);

internalAcquireVMAccess(currentThread);
jfrEvent->loadedClassCount = vm->loadedClassCount;
}
}

Expand Down
3 changes: 3 additions & 0 deletions runtime/vm/jvminit.c
Original file line number Diff line number Diff line change
Expand Up @@ -1179,6 +1179,9 @@ initializeJavaVM(void * osMainThread, J9JavaVM ** vmPtr, J9CreateJavaVMParams *c
vm->internalVMLabels = (J9InternalVMLabels*)-1001;
vm->cInterpreter = J9_BUILDER_SYMBOL(cInterpreter);
vm->threadDllHandle = createParams->threadDllHandle;
#if defined(J9VM_OPT_JFR)
vm->loadedClassCount = 0;
#endif /* defined(J9VM_OPT_JFR) */

#if JAVA_SPEC_VERSION >= 19
/* tid 1 will be use by main thread, first usable tid starts at 2 */
Expand Down