defender methods analysis, classloading and java.lang.StackOverflowError

[Sergey Kuksenko]

On 03/14/2012 07:55 PM, Keith McGuigan wrote:
> Hi Sergey -
>
> BTW, I'm not sure if GCC has any fault in this situation -- the way it's
> currently coded, we need to have size in parseClassFile()'s frame for
> the entirety of the GenericAnalyzer object.
Iti's philosophical question. Function frame may be dynamic, but I 
didn't hear about production compilers with that feature.And since 
parseClassFile()
> calls itself recursively....
>
> I suspect replacing the stack-allocation with resource-allocation would
> resolve the problem too:
>
> {
> GenericAnalyzer* ga = new GenericAnalyzer(...);
> ga->run(...)
> }
>
> ... because then we'll use less room in parseClassFile()'s stack.
We may try this.
But I suspect not only impact of GenericAnalyzer size. The problem may 
be caused by aggressive inline and corresponding stack frame expansion.

ParseClassFile() is too big - ~900 lines.
IMHO the method should be split.
>   But I
> don't think that's going to explain the ~8K/frame difference you're
> seeing.  That's probably coming from within the GenericAnalyzer itself
> (which does use recursion too).
No. The branch "if (has_default_methods && !access_flags.is_interface()) 
{" is never executed for that test.

> - Keith
>
> On 3/14/2012 11:17 AM, Sergey Kuksenko wrote:
>> On 03/14/2012 07:08 PM, Sergey Kuksenko wrote:
>>> Hi All,
>>>
>>> It is not a big secret that HotSpot do classloading of
>>> superclasses/superinterfaces recursively.
>>> That means - for any fixed stack size there is classes/interfaces
>>> chain which causes java.lang.StackOverflowError
>>> For example, on Linux x86-32, chain of 512 interfaces needs at least
>>> 2.5Megabytes stack size using jdk7u2 (default stack size == 320k).
>>> Maybe it is not a good idea to do classloading with recursion, but it
>>> is a topic for another mail list.
>>>
>>> Let's back to defenders.
>>> I was playing with interface chains. All interfaces (except the first)
>>> are empty. There are no defender methods here.
>>> As result I've got on Linux x86-32:
>>> - jdk7u2 requires ~4.8K per interface
>>> - jdk8 requires ~5K per interface
>>> - lambda build requires ~13K per interface.
>>>
>>> Lambda's HotSpot requires 2.5 larger stack size settings than jdk8.
>>> This is really significant change. Jdk7&  8 with default stack size
>>> (Linux32) may load chain of ~60 classes/interfaces, but lambda may
>>> load chain only of ~20 classes/interfaces. The source of that is
>>> declaration "GenericAnalyzer ga;" in the
>>> "ClassFileParser::parseClassFile" method and it doesn't matter that
>>> the declaration is inside never executed scope/block.
>>> We may say "thank you" to gcc. I didn't check, but expecting the
>>> similar problem for other C++ compilers and platforms.
>>>
>>> I've attached patch which solves that problem. Simple extracting
>>> couple lines of code into separate method bring back lambda to jdk8
>>> stack size requirements.
>>>
>> It looks like I lost the attachment, trying to repeat.
>>
>>
>> ------------------------------
>> diff -r 108d1e7220ee src/share/vm/classfile/classFileParser.cpp
>> --- a/src/share/vm/classfile/classFileParser.cpp Thu Feb 02 01:53:12
>> 2012 -0500
>> +++ b/src/share/vm/classfile/classFileParser.cpp Tue Mar 13 19:24:51
>> 2012 +0400
>> @@ -3396,13 +3396,9 @@
>> #endif
>>
>> if (has_default_methods&&  !access_flags.is_interface()) {
>> - ResourceMark rm(THREAD);
>> -
>> - GenericAnalyzer ga;
>> - ga.run(
>> - class_name, this_klass, super_klass,&all_mirandas,
>> - methods, methods_annotations, methods_parameter_annotations,
>> - methods_default_annotations, CHECK_(nullHandle));
>> + analyze_defaults(class_name, this_klass, super_klass,&all_mirandas,
>> + methods, methods_annotations, methods_parameter_annotations,
>> + methods_default_annotations, CHECK_(nullHandle));
>> }
>>
>> // Miranda methods
>> @@ -3523,6 +3519,22 @@
>> return this_klass;
>> }
>>
>> +void ClassFileParser::analyze_defaults(
>> + Symbol* class_name, instanceKlassHandle klass,
>> + instanceKlassHandle super, GrowableArray<methodOop>* mirandas,
>> + objArrayHandle methods, objArrayHandle annotations,
>> + objArrayHandle annot_params, objArrayHandle annot_defaults, TRAPS){
>> +
>> + ResourceMark rm(THREAD);
>> +
>> + GenericAnalyzer ga;
>> + ga.run(
>> + class_name, klass, super, mirandas,
>> + methods, annotations, annot_params,
>> + annot_defaults, THREAD);
>> +
>> +}
>> +
>>
>> unsigned int
>> ClassFileParser::compute_oop_map_count(instanceKlassHandle super,
>> diff -r 108d1e7220ee src/share/vm/classfile/classFileParser.hpp
>> --- a/src/share/vm/classfile/classFileParser.hpp Thu Feb 02 01:53:12
>> 2012 -0500
>> +++ b/src/share/vm/classfile/classFileParser.hpp Tue Mar 13 19:24:51
>> 2012 +0400
>> @@ -139,6 +139,12 @@
>> void parse_classfile_signature_attribute(constantPoolHandle cp,
>> instanceKlassHandle k, TRAPS);
>> void parse_classfile_bootstrap_methods_attribute(constantPoolHandle cp,
>> instanceKlassHandle k, u4 attribute_length, TRAPS);
>>
>> + void analyze_defaults(
>> + Symbol* class_name, instanceKlassHandle klass,
>> + instanceKlassHandle super, GrowableArray<methodOop>* mirandas,
>> + objArrayHandle methods, objArrayHandle annotations,
>> + objArrayHandle annot_params, objArrayHandle annot_defaults, TRAPS);
>> +
>> // Annotations handling
>> typeArrayHandle assemble_annotations(u1* runtime_visible_annotations,
>> int runtime_visible_annotations_length,
>>
>>
>>
>>


-- 
Best regards,
Sergey Kuksenko