Re: Patch: FYI: More efficient ResourceBundle calls

[Dalibor Topic]

Ni Bryce,

Bryce McKinlay wrote:
> Dalibor Topic wrote:

> > 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.

As the performance problem is in calling-classloader check,... would it 
be securely possible to cache the result of a calling-classloader check?

> > 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?

I think that the bootstrap class loader, the extension class loader and 
the system class loader can be different classes, so I think it should 
be possible to load a core class [1] with a class loader that's not 
system class loader. Chances are that it would be a class loader on the 
system class loader's delegation chain, though :)

Another opportunity seems to be through setting java.system.class.loader 
property to say, OwnSystemClassLoader. Then you could have a class 
loader chain that looks like:

*-bootstrap-extension-interimclassloader-ownsystemclassloader

Hm, I think you're right that it may not matter, as the class loaders 
are on the ownsystemclassloader's delegation chain, so as long as 
ownsystemclassloader follows the API (delegate to parent first!) and/or 
defineClass checks for java.*, we're safe.

But I still think there is a small bug left:)

I think in the Window patch, the actual ressource name can be passed via 
 a public api, so I'm not sure if automatically using the system class 
loader may not break things there. Consider the case where the ressource 
bundle is loadable through the calling classes' class loader, but not 
through the system class loader.

The other uses appear to be restricted to 
"gnu.java.locale.LocaleInformation", and "gnu.java.locale.Calendar", so 
those resources look like a safe bet for the system class loader.

cheers,
dalibor topic

[1] Uh, what packages precisely are we talking about? java.*, right? The 
classpath docs ocassionally use 'core classes' to denote all of 
classpath, so I am a little confused. The JDK 1.4.2 API for 
ClassLoader.defineClass says that only bootstrap can create java.* 
classes, to I take java.* should mean 'core'.