best practices for testing JVM changes that may interact with CDS?

[Coleen Phillmore]

Hi,

Some comments below.

On 7/18/2013 8:51 PM, Ioi Lam wrote:
> On 07/18/2013 03:48 PM, John Rose wrote:
>> On Jul 18, 2013, at 2:15 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>>> John, for vtable, do you mean this code?
>>>
>>> void InstanceKlass::restore_unshareable_info(TRAPS) {
>>>    ...
>>>    for (int index2 = 0; index2 < num_methods; ++index2) {
>>>      methodHandle m(THREAD, methods->at(index2));
>>>      m()->link_method(m, CHECK);
>>>      // restore method's vtable by calling a virtual function
>>>      m->restore_vtable();    /// <<<<<< here??
>>>    }
>> Perhaps I'm thinking of that bit, which concerns C++ vtables, and 
>> doesn't overlap with Klass-to-Method vtables.
>>
>> Am I just imagining an interaction between CDS and tables containing 
>> Method* pointers?
>
> John,
>
> You're right, CDS just messes with C++ vtables, not Java vtables.
>
> The C++ vtables of these types are patched:
>
> void Universe::init_self_patching_vtbl_list(void** list, int count) {
>   int n = 0;
>   { InstanceKlass o;          add_vtable(list, &n, &o, count); }
>   { InstanceClassLoaderKlass o; add_vtable(list, &n, &o, count); }
>   { InstanceMirrorKlass o;    add_vtable(list, &n, &o, count); }
>   { InstanceRefKlass o;       add_vtable(list, &n, &o, count); }
>   { TypeArrayKlass o;         add_vtable(list, &n, &o, count); }
>   { ObjArrayKlass o;          add_vtable(list, &n, &o, count); }
>   { Method o;                 add_vtable(list, &n, &o, count); }
>   { ConstantPool o;           add_vtable(list, &n, &o, count); }
> }
>
> I think whole mechanism is gross and should be cleaned up 
> (https://jbs.oracle.com/bugs/browse/JDK-8005165), but I never got 
> around doing it. In any case, if you add/remove C++ virtual methods in 
> these classes, I doubt anything would break. But you never know :-)

There's a fixed number of expected vtable entries for the metadata 
types.   If you add more than that, it'll break in a sort of ugly way.  
I think we tried to add an assert for that though.

>
> Incidentally, I tried removing the m->restore_vtable() call that I 
> marked above and nothing seems to break. It seems redundant.
>

This function was to eagerly call a virtual method for Method* so that 
the C++ vtable is restored.    There was  a reason this needed to be 
eager.   I think there is code in dtrace and pstack (libjvm_db.c) that 
matches __vtbl_xMethod to see if a pointer is a Method pointer.   There 
should be a better comment there though.

Coleen
> - Ioi
>
>
>
>
>
>
>
>
>> — John
>