Request for review 8007320: NPG: move method annotations

[Jiangli Zhou]

Hi Coleen,

Looks great! It also simplified merge_in_new_methods(). :)

I wonder if the AnnotationArray pointers could be eliminated by 
embedding the raw annotation data in ConstMethod directly. The 
ConstMethod::_flags could still be kept as 8-bit, with one bit to 
indicate if the method has any type of the annotations. The embedded raw 
annotation data would look like following in the table. Each raw 
annotation data is preceded by it's type and size, with type being 
1-byte and size being 4-byte. That way there is no need to explicitly 
free the method annotation arrays, and also reduces fragmentation. There 
is no extra cost for the 4-byte 'size' in the embedded table for each 
annotation, as Array would also has an int field for the size.

...

|------------------------------------------------------|
| generic signature index (u2)                         |
|  (indexed from start of constMethodOop)              |
|------------------------------------------------------|
| method annotations:                                  |
| type|size|method_annotations raw data|type|size|     |
| parameter_annotations raw data|...                   |
--------------------------------------------------------

Thanks,
Jiangli

On 01/31/2013 02:12 PM, Coleen Phillimore wrote:
> Summary: allocate method annotations and attach to ConstMethod if present
>
> From the bug report:
>
> This is related to 8003419. The annotations are allocated in Metaspace 
> during class file parsing and pointers to them are carried around 
> through the parser. In order to clean these up, you'd have to collect 
> these pointers somewhere so they can be cleaned up if the parse fails.
>
> Instead, attach method annotations to the constMethod so that they can 
> be cleaned up if the ConstMethod has to be cleaned up.
>
> If any annotations exists for any method, an Array<u1> is created for 
> that method, but it's put into an Array<Array<u1>*> (an array of these 
> arrays) where there's an entry for each method in the klass, so the 
> other methods would have a pointer allocated for it whether needed or 
> not. There are 3 of these array of arrays in the type Annotations, and 
> an Annotations* object for type annotations, which are so far created 
> infrequently.
>
> The fix is to move the 4 types of method annotations to embedded 
> pointers in the ConstMethod if they are needed and add a flag to 
> indicate whether they are present. You could embed the annotations 
> directly, but the length has to be pulled out as an 'int' from 
> unaligned storage, and the bit math is getting to be too fragile 
> without a redesign.
>
> Also, some code was refactored in parse_method() and the code that 
> sorted annotation arrays to match sorted method arrays isn't needed 
> anymore.  This is in a couple of places, including defaultMethods and 
> RedefineClasses.
>
> The real purpose of this change is to make the annotations allocated 
> in Metaspace easier to clean up if class file parsing fails, but 
> hopefully has some cleanup and space saving benefits as well.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8007320/
> bug link at http://bugs.sun.com/view_bug.do?bug_id=8007320 (just filed 
> it so might not be available yet)
>
>
> Tested with serviceability tests (nsk.sajdi.testlist, 
> vm.tmtools.testlist),  jtreg tests for annotations, including the new 
> type annotation tests, jtreg tests for java/lang/instrument, 
> nsk.parallel_class_loading.testlist and vm.quick.testlist (which is 
> the same as full).
>
> Thanks,
> Coleen
>
>