RFR: 8293979: Resolve JVM_CONSTANT_Class references at CDS dump time [v3]

[Coleen Phillimore]

On Tue, 18 Oct 2022 06:51:15 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> src/hotspot/share/cds/classPrelinker.cpp line 59:
>> 
>>> 57: ClassPrelinker::ClassPrelinker() {
>>> 58:   assert(_singleton == NULL, "must be");
>>> 59:   _singleton = this;
>> 
>> I'm not sure why you have this?
>
> I was trying to put the states of the ClassPrelinker in its instance fields, but this is probably confusing. I'll change ClassPrelinker to AllStatic instead.

It doesn't look like you did this.

>> src/hotspot/share/cds/classPrelinker.cpp line 205:
>> 
>>> 203: 
>>> 204: #if INCLUDE_CDS_JAVA_HEAP
>>> 205: void ClassPrelinker::resolve_string(constantPoolHandle cp, int cp_index, TRAPS) {
>> 
>> Is this function only needed in this cpp file? Can you make it static and defined before its caller? Are there other functions where this is true?  Does it need to be a member function of ClassPrelinker?
>
> I prefer to make such methods private, so it can access other private members of the same class.

but this doesn't reference other private members of the class. The nice thing about it being a static non-member function is that you can easily see this without looking anywhere else. Also, it can be defined above the caller, which is where it's easy to find.
It's something that varies with developers but I like the style where you read a new file and see little function, little function, etc, leading up to the big function that calls them all.

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

PR: https://git.openjdk.org/jdk/pull/10330