Re: Patch: FYI: More efficient ResourceBundle calls

[Bryce McKinlay]

Dalibor Topic wrote:

> > I think the solution here is to pass the system class loader. This 
> > should always be correct for these bootstrap classes. It doesn't 
> > solve the performance issues for cases where a security manager is 
> > present, since a check will be performed by the getSystemClassLoader 
> > call - however its better than what we had before and certainly 
> > better than my incorrect patch ;-)
>
>
> Ah, the curse of getting microoptimizations right ;)


I don't really consider this to be a micro-optimization. In libgcj it 
causes pretty severe performance problems. You could argue that this is 
because libgcj's calling-classloader check is too slow, and you'd be 
right, but I would imagine that in most VMs, stack-walking checks are 
relatively slow. Given that code like "new Date()" is very frequently 
executed in many apps, I think its  important to make this as fast as we 
reasonably can.

Of course, I will concede that correctness must take presidence over 
performance :)

> I think the patch is still wrong, in cases where one uses a Calendar 
> class that's not loaded via the system class loader (like the 
> bootstrap class loader, extension class loader, etc [1] :).


Hmm - but can this ever happen? Is it possible to define a core class 
with a non-system class loader? In any case, it seems to me that it 
should always be correct to load Locale resources etc with the system 
class loader - or is there a situation I havn't considered?

Regards

Bryce