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

Retained size by ClassLoaders #35

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Karina5005
Copy link

No description provided.

@bibaev bibaev requested review from bibaev and nikita-nazarov May 13, 2022 15:03
}

jlongArray RetainedSizeByClassLoadersAction::executeOperation(jobjectArray classLoadersArray) {
std::vector <jlong> result;
Copy link
Contributor

Choose a reason for hiding this comment

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

An extra space

handleError(jvmti, err, "Could not estimate retained size by classLoaders");
return env->NewLongArray(0);
}
std::cout << "HEAP SIZE: " << getHeapSize() << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since memory agent is a library, it would be better not to pollute stdout, but instead return the result in the format of nested arrays: [[class loaders sizes as jlongs], [other classes size]]

private:
jlongArray executeOperation(jobjectArray classLoadersArray) override;
jvmtiError getRetainedSizeByClassLoaders(jobjectArray classLoadersArray, std::vector<jlong> &result);
jvmtiError tagObjectsByClassLoader(jclass *classes, jint* cnt, jobject classLoader, jsize classLoaderIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Asterisks should stick to the variable name

jvmtiError getRetainedSizeByClassLoaders(jobjectArray classLoadersArray, std::vector<jlong> &result);
jvmtiError tagObjectsByClassLoader(jclass *classes, jint* cnt, jobject classLoader, jsize classLoaderIndex);
jlong getHeapSize();
jlong tagOtherObjects(jclass *classes, jint* cnt, jobjectArray classLoadersArray);
Copy link
Contributor

Choose a reason for hiding this comment

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

Asterisks should stick to the variable name

@@ -49,6 +49,15 @@ jobjectArray toJavaArray(JNIEnv *env, std::vector<jobject> &objects) {
return res;
}

jobjectArray toJavaArray(JNIEnv *env, std::vector <jclass> &objects) {
Copy link
Contributor

Choose a reason for hiding this comment

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

An extra space

@@ -97,6 +97,14 @@ public synchronized <T> T[] getAllReachableObjects(Object startObject, Class<T>
return resultArray;
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

The formatting is a little off here

result.resize(env->GetArrayLength(classLoadersArray) + 1);
err = IterateThroughHeap(JVMTI_HEAP_FILTER_UNTAGGED, nullptr, visitObject, result.data(),
"calculate retained size");
return err;
Copy link
Contributor

Choose a reason for hiding this comment

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

return IterateThroughHeap(JVMTI_HEAP_FILTER_UNTAGGED, nullptr, visitObject, result.data(),
                             "calculate retained size");

err = jvmti->GetLoadedClasses(&cnt, &classes);
std::cout << cnt << std::endl;
for (jsize i = 0; i < env->GetArrayLength(classLoadersArray); i++) {
jvmtiError err = tagObjectsByClassLoader(classes, &cnt, env->GetObjectArrayElement(classLoadersArray, i), i);
Copy link
Contributor

Choose a reason for hiding this comment

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

if (!isOk(err)) return err

Copy link
Contributor

Choose a reason for hiding this comment

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

Also there's no need to declare a new variable here

for (jsize i = 0; i < env->GetArrayLength(classLoadersArray); i++) {
jvmtiError err = tagObjectsByClassLoader(classes, &cnt, env->GetObjectArrayElement(classLoadersArray, i), i);
}
tagOtherObjects(classes, &cnt, classLoadersArray);
Copy link
Contributor

Choose a reason for hiding this comment

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

err = tagOtherObjects(classes, &cnt, classLoadersArray);
if (!isOk(err)) return err

jlong tagOtherObjects(jclass *classes, jint* cnt, jobjectArray classLoadersArray);
};

jint JNICALL visitObject(jlong classTag, jlong size, jlong *tagPtr, jint length, void *userData);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function should not be declared here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants