Supporting inline types in Java debugger

[David Simms]

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