On Thu, 25 Jan 2024 23:05:19 GMT, Alex Menkov <amen...@openjdk.org> wrote:
> The fix adds new test for FollowReferences JVMTI function to verify
> correctness of reported field indexes.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 93:
> 91:
> 92: /*
> 93: Per jvmtiHeapReferenceInfoField spec:
I think you should also include mention that this is for
JVMTI_HEAP_REFERENCE_STATIC_FIELD.
Indentation should be fixed to match our C++ comment style, and the "bullets"
from the spec should be included to make it clearer that these are two lists of
steps.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 108:
> 106: where n is the count of the fields in all the superinterfaces of I.
> 107:
> 108: 'Klass' struct contains all required data to calculate field indeces.
"indices"
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 153:
> 151:
> 152: /*
> 153: For each test object 'Object' struct is created and pointer to it set as
> a tag for jobject.
Suggestion:
For each test object, the 'Object' struct is created and a pointer to it is set
as the jobject's tag.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 225:
> 223:
> 224: // Explores all interfaces implemented by 'klass' sorting out duplicates
> 225: // and store them in the 'arr' starting from 'index'.
Suggestion:
// Explores all interfaces implemented by 'klass', sorts out duplicates,
// and stores the interfaces in the 'arr' starting from 'index'.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 254:
> 252:
> 253: // And explore its superinterfaces.
> 254: count += fill_interfaces(arr, index + count, env, interfaces[i]);
I assume here we are always dealing with test classes that are known not to be
deeply nested.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 348:
> 346:
> 347: jclass obj_klass = env->GetObjectClass(obj);
> 348:
Unnecessary newline.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 350:
> 348:
> 349: Klass* klass = Klass::explore(env, obj_klass);
> 350:
Unnecessary newline.
test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp
line 492:
> 490: Java_FieldIndicesTest_prepare(JNIEnv *env, jclass cls, jobject testObj) {
> 491: Object::explore(env, testObj);
> 492: fflush(0);
stdout?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473643948
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473644156
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473646542
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473647766
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473649198
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473651074
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473651163
PR Review Comment: https://git.openjdk.org/jdk/pull/17580#discussion_r1473650827
