Re: [cp-patches] RFC: Adding a cache for locale information

[Guilhem Lavaux]

Mark Wielaard wrote:
> Hi,
>
> On Sun, 2005-12-11 at 14:20 +0100, Guilhem Lavaux wrote:
>
> > Using ant we may have discovered that multiple creation of a 
> > SimpleDateFormat object can lead to a big slowdown. I am proposing the 
> > addition of a simple cache system. LocaleCache would cache 
> > ResourceBundle objects and some parsed strings in a WeakHashMap. If it 
> > is needed LocaleCache will create these objects and in the other case 
> > extract them directly from the cache.
> >
> > This patch shows DateFormatSymbols can be easily modified to follow this 
> > scheme.
>
>
> If you are really seeing big speedups then this makes sense. Do you have
> an indication of how much time/resources are involved?

I first wanted to test on ant's build itself but it hasn't improved 
anything. I (or mnemoc) will test eclipse's build...

> Some comments on this patch.
> - The cache can be accessed from multiple Threads so all access should
> be synchronized.

Ok. No problem.

> - When does Locale.toString() every return the empty String?

If both language and variants are empty. We should never use this case 
though.

> - In getZoneStrings() you now catch/handle the case of a
> MissingResourceException. Why wasn't this needed before? Does this
> change the semantics?

I haven't changed the semantics. The catch was already there. By the 
way, I will also remove the code from DateFormatSymbols in the next 
patch as it becomes unused.

> - The use of ClassLoader.getSystemClassLoader() cannot be done in all
> context. If you know this is safe here (I believe it is) then you can
> use a doPrivilieged() call here. Or maybe even add a new Action for
> getting the System Class Loader in gnu.java.security.action since it is
> done more often in the core classes. (This was wrong in the original
> code so isn't a new issue. If you decide not to fix it then please file
> a bug report for it.)

Ok. I will fix this...

> Minor nitpicks:
>
> - There are multiple lines > 80 chars.
> - There should be spaces around all operators like + or =.

Ok.

I'll post a new patch asap.

Cheers,

Guilhem.