Supporting inline types in Java debugger

[Frederic Parain]

David,

Thank you for the review.

Fred


> On Oct 9, 2019, at 04:01, David Simms <david.simms at oracle.com> wrote:
> 
> 
> Nice, looks good, push it..
> 
> On 2019-10-08 21:42, Frederic Parain wrote:
>> Thank you for looking at these changes!
>> Comments are inlined below.
>> 
>>> On Oct 8, 2019, at 06:33, David Simms <david.simms at oracle.com> wrote:
>>> 
>>> 
>>> Sorry for the delayed reply...
>>> 
>>> src/hotspot/share/prims/jni.cpp:2075
>>> 
>>> Isn't this dance repeated some where else (interpreter runtime) ?  Is it worth refactoring, maybe moving to instanceKlass ?
>>> 
>> Good point. I’ve added two methods to ValueKlass:
>> 
>>   oop read_flattened_field(oop obj, int offset, TRAPS);
>>   void write_flattened_field(oop obj, int offset, oop value, TRAPS);
>> 
>> and I’ve refactored all places where flattened fields were read from
>> or written to in runtime code (interpreter runtime: getfield/withfield/putfield,
>> JNI, and Unsafe).
> 
> 
> Encapsulated the "empty value" handling, nice.
> 
> 
>>> General:
>>> 	• Testing is there any automated jdb testing ?
>>> 		• I assume this is actually a first pass for Daniil to get initial manual testing up ?
>> I guess he did manual testing (I did the same).
>> 
>> Testing the changes in this patch would require to:
>>   - update jdb tests in test/hotspot/jtreg/vmTestbase/nsk/jdb
>>   - update JDI tests in test/hotspot/jtreg/vmTestbase/nsk/jdi
>>   - update JDWP tests in test/hotspot/jtreg/vmTestbase/nsk/jdwp
>>   - update JVMTI tests in test/hotspot/jtreg/vmTestbase/nsk/jvmti
>>   - update JNI tests  (they are spread over several places)
>> 
>> I would prefer to wait until the user model is finalized, and we already
>> have a good idea how all the different specifications will be updated
>> to support inline types. All these components of the JPDA are highly
>> dependent on signatures, and we are expecting big changes to happen in
>> this area.
>> 
>> 
>>> 	• Draft spec changes will need to be tracked somewhere
>>> 		• New RFE ?
>> I could create a new RFE, but as stated above, I don’t think we have
>> all the data yet to start drafting the new JPDA specs.
>> 
>>> Otherwise, looks good as is.
>>> 
>>> 
>> The new webrev is here:
>> 
>> http://cr.openjdk.java.net/~fparain/jpda/webrev.01/index.html
>> 
>> 
>> Thank you,
>> 
>> Fred
>> 
> Agreed, push it, this enables further work, spec and test work noted TODO for project...
> 
> 
> Thank you Frederic !
> 
> /D
> 
>