Re: Request for review:8020309:Remove InstanceKlass::_cached_class_file_len and record the length together with the cached class file bytes

[Jiangli Zhou]

On 07/15/2013 01:58 PM, Jiangli Zhou wrote:

Hi Serguei,

Thanks for the review! I've rerun all following tests and JPTR:

    jdk/test/java/lang/instrument
    jdk/test/com/sun/jdi
    nsk.jvmti
    nsk.jdi
    nsk.hprof

And vm.quick.testlist as well.

Thanks,
Jiangli

Thanks,
Jiangli

On 07/15/2013 01:07 PM, serguei.spit...@oracle.com wrote:

Hi Jiangli,

It looks good, thank you for the update!
It makes sense to also run the JTREG java/lang/instrument tests.

Thanks,
Serguei

On 7/12/13 1:55 PM, Jiangli Zhou wrote:

Hi Ioi and Serguei,

Here is the updated the webrev:

http://cr.openjdk.java.net/~jiangli/8020309/webrev.01/.

I've retested with nsk.jvmti, nsk.jdi and nsk.hprof. I'm rerunning the JPRT and rest of the tests.

Thanks,
Jiangli

On 07/11/2013 01:14 AM, serguei.spit...@oracle.com wrote:

Hi Jiangli,

I like the Ioi's sugestion.

It will help to avoid the manipulations with offsets that look as unnecessary complexity.

Thanks,
Serguei


On 7/10/13 6:39 PM, Jiangli Zhou wrote:

Hi Ioi,

That's good suggestion. Let me look into it.

Thanks!

Jiangli

On 07/10/2013 05:53 PM, Ioi Lam wrote:

Hi Jiangli,

I think it's better to move the logic of deciding the length into JVMTI. E.g., have something like

struct JvmtiCachedClassFileData {
   int _length;
   unsigned char data[1];
};

void instanceKlass::set_jvmti_cached_class_file_data(JvmtiCachedClassFileData *data);

- Ioi

On 07/10/2013 04:58 PM, Jiangli Zhou wrote:

Hi,

Please review the webrev for JDK-8020309 <https://jbs.oracle.com/bugs/browse/JDK-8020309>:

http://cr.openjdk.java.net/~jiangli/8020309/webrev.00/

The _cached_class_file_bytes and _cached_class_file_len are not set/used if no jvmti agent is attached. The _cached_class_file_len field can be eliminated and the file length can be recorded as part of the cached class data in _cached_class_file_bytes for redefined class. The above change allocates extra 4bytes when allocating memory for cached class file and stores the length as the first 4bytes and the file data as the rest. It saves 4 bytes per class. For classes redefined, the memory usage is the same.

Tested with following tests and JPRT:

    jdk/test/java/lang/instrument
    jdk/test/com/sun/jdi
    nsk.jvmti
    nsk.jdi
    nsk.hprof

Thanks,
Jiangli