RFR: 8313638: Add test for dump of resolved references [v2]

[Ioi Lam]

On Tue, 12 Sep 2023 18:50:22 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> The change in [JDK-8306582](https://bugs.openjdk.org/browse/JDK-8306582) revealed that the state of the resolved references array is not checked in the CDS archive. This patch adds a test to ensure that the resolved references array is correct whether the application is archived or not.
>
> Matias Saavedra Silva has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fixed spacing
>  - Calvin comments

The test logic looks good to me. I think the comments can be improved.

test/hotspot/jtreg/runtime/cds/appcds/sharedStrings/ResolvedReferencesNotNullTest.java line 47:

> 45:         // If ResolvedReferencesTestApp is not archived, the resolvedReferences array should only contain
> 46:         // the objects fooString and barString since they are static. The object
> 47:         // quxString should NOT be in the array since the method that returns it is not called

I think this comment should be moved into ResolvedReferencesWb.java, so it's closed to the actual test logic.

test/hotspot/jtreg/runtime/cds/appcds/sharedStrings/ResolvedReferencesWb.java line 61:

> 59:         // them inside the resolvedReferences array. The strings fooString and
> 60:         // barString must have been found but not quxString unless ResolvedReferencesTestApp is archived.
> 61:         if (isArchived) {

I think the comment would be clear if you break it down in the two case, with something like this:


if (isArchived) {
    // CDS eagerly resolves all the string literals in the ConstantPool. At this point, all
    // three strings should be in the resolvedReferences array. 
} else {
    // If the class is not archived, the string literals in the ConstantPool are resolved
    // on-demand. At this point, ResolvedReferencesTestApp::<clinit> has been executed
    // so the two strings used there should be in the resolvedReferences array. 
    // ResolvedReferencesTestApp::qux() is not executed so "quxString"
    // should not yet be resolved.
}

-------------

PR Review: https://git.openjdk.org/jdk/pull/15686#pullrequestreview-1623109005
PR Review Comment: https://git.openjdk.org/jdk/pull/15686#discussion_r1323550590
PR Review Comment: https://git.openjdk.org/jdk/pull/15686#discussion_r1323559479