Request for review: 7150058: Allocate symbols from null boot loader to an arena for NMT

[Coleen Phillimore]

On 3/8/2012 12:51 PM, Tom Rodriguez wrote:
> On Mar 8, 2012, at 7:55 AM, Coleen Phillimore wrote:
>
>> Please review this change to allocate symbols from the boot class loader into a  global arena.   This change reduces reporting traffic at startup for the native memory tracking project in development.  It is also useful for class data sharing in the permgen elimination project, also in development.   I had to increase the SharedMiscSize because ResourceObj in debug mode is 2 extra pointers.
> I think the decision about where a symbol is allocated should be inside of operator new instead of up in SymbolTable.cpp.  Why not pass the class loader all the way through?

I thought this is a lot cleaner but the lookup() calls call basic_add() 
and they don't have a class loader passed down.   We don't really want a 
null class loader in this case because many lookup calls are for 
temporary symbols, or for symbols constructed to find a class.   These 
temporary symbols would be good to remove.
> Why is there a path where c_heap is being forced?
>
> +  // Create a new symbol.
> +  Symbol* sym = allocate_symbol(name, len, true, CHECK_NULL);
> +  assert(sym->equals((char*)name, len), "symbol must be properly initialized");

This is the basic_add() case that comes from lookup().  There is one 
path below where we know the class loader.   I started to hook it up, 
then decided that it wasn't worth changing all the basic_add() calls to 
pass "true" in the cases where we don't know what class loader the 
symbol is from.
> Maybe this isn't wired up?
>
> +      bool c_heap = class_loader() != NULL;
>         Symbol* sym = table->basic_add(index, (u1*)names[i], lengths[i],
> -                                       hashValues[i], CHECK);
> +                                     hashValues[i], CHECK);

I didn't end up wiring this up, see above.  I deleted this line.  This 
case is when we have a list of symbols and for some reason the batch 
basic add fails.  The only case that would happen is if we couldn't get 
a new arena, so it sort of makes sense to individually allocate the 
symbols in C heap.

Thanks,
Coleen
> tom
>
>> open webrev at http://cr.openjdk.java.net/~coleenp/symbol-arena
>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7150058
>>
>> Thanks,
>> coleen
>>