Re: Patch: FYI: More efficient ResourceBundle calls

[Bryce McKinlay]

Mark Wielaard wrote:

> On Tue, 2004-06-15 at 16:56, Bryce McKinlay wrote:
>  
>
> > I'm checking in this patch from libgcj. It changes various
> > ResourceBundle.getBundle() calls to use the 3-argument form which
> > includes a ClassLoader parameter. This call is  more efficient because
> > it means getBundle() does not have to walk the stack to find the
> > calling classloader. This should speed up some Date/Calendar
> > operations which are currently quite slow due to repeated calling
> > classloader checks. Other VMs may implement the classloader check
> > faster than libgcj does currently, however it will presumably always
> > be faster to pass the classloader explicitly - so please use the
> > 3-argument form when adding new getBundle() calls.
> >    
>
> This looks a bit bogus to me.
>
>  
>
> > -    return ResourceBundle.getBundle(bundleName, locale);
> > +    return ResourceBundle.getBundle(bundleName, locale,
> > +      Calendar.class.getClassLoader());
> >    
>
> In every case you call getClassLoader() on a bootstrap class. So you
> already know that it will be null. So you could have just passed in null
> as loader. But ResourceBundle.getBundle() is documented to throw a
> NullPointerException when any of its arguments is null. It seems we have
> a bug because we don't throw a NullPointerException (we used to throw it
> when we used HashTable, but we switched to HashMap which allows null
> values), but the kaffe implementation does (IMHO correctly) throw a
> NullPointerException.
>  

OK, in libgcj, getClassLoader() never returns null, but the spec does 
say that implementations may use null to represent the bootstrap 
classloader. Since the classloader argument to getBundle() may not be 
null, so we have a problem.

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 ;-)

I'm checking in this.

Regards

Bryce

2004-06-15  Bryce McKinlay  <address@hidden>

        * java/util/Calendar.java: Use getSystemClassLoader as argument for
        ResourceBundle.getBundle() calls.
        * java/util/GregorianCalendar.java: Likewise.
        * java/util/Currency.java: Likewise.
        * java/text/BreakIterator.java: Likewise.
        * java/text/Collator.java: Likewise.
        * java/text/DateFormat.java: Likewise.
        * java/text/DateFormatSymbols.java: Likewise.
        * java/text/DecimalFormatSymbols.java: Likewise.
        * java/text/NumberFormat.java: Likewise.
        * java/awt/Window.java: Likewise.

Index: java/util/Calendar.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/Calendar.java,v
retrieving revision 1.20
diff -u -r1.20 Calendar.java
--- java/util/Calendar.java     14 Jun 2004 15:51:36 -0000      1.20
+++ java/util/Calendar.java     15 Jun 2004 22:37:11 -0000
@@ -377,7 +377,7 @@
   private static ResourceBundle getBundle(Locale locale) 
   {
     return ResourceBundle.getBundle(bundleName, locale,
-      Calendar.class.getClassLoader());
+      ClassLoader.getSystemClassLoader());
   }
 
   /**
Index: java/util/GregorianCalendar.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/GregorianCalendar.java,v
retrieving revision 1.21
diff -u -r1.21 GregorianCalendar.java
--- java/util/GregorianCalendar.java    14 Jun 2004 15:51:36 -0000      1.21
+++ java/util/GregorianCalendar.java    15 Jun 2004 22:37:11 -0000
@@ -91,7 +91,7 @@
   private static ResourceBundle getBundle(Locale locale) 
   {
     return ResourceBundle.getBundle(bundleName, locale,
-      GregorianCalendar.class.getClassLoader());
+      ClassLoader.getSystemClassLoader());
   }
 
   /**
Index: java/util/Currency.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/Currency.java,v
retrieving revision 1.3
diff -u -r1.3 Currency.java
--- java/util/Currency.java     14 Jun 2004 15:51:36 -0000      1.3
+++ java/util/Currency.java     15 Jun 2004 22:37:11 -0000
@@ -55,7 +55,7 @@
   {
     this.locale = loc;
     this.res = ResourceBundle.getBundle ("gnu.java.locale.LocaleInformation", 
-      locale, Currency.class.getClassLoader());
+      locale, ClassLoader.getSystemClassLoader());
   }
 
   /**
Index: java/text/BreakIterator.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/BreakIterator.java,v
retrieving revision 1.8
diff -u -r1.8 BreakIterator.java
--- java/text/BreakIterator.java        14 Jun 2004 15:51:36 -0000      1.8
+++ java/text/BreakIterator.java        15 Jun 2004 22:37:11 -0000
@@ -136,7 +136,7 @@
       {
        ResourceBundle res
          = ResourceBundle.getBundle("gnu.java.locale.LocaleInformation",
-                                    loc, BreakIterator.class.getClassLoader());
+                                    loc, ClassLoader.getSystemClassLoader());
        className = res.getString(type);
       }
     catch (MissingResourceException x)
Index: java/text/Collator.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/Collator.java,v
retrieving revision 1.14
diff -u -r1.14 Collator.java
--- java/text/Collator.java     14 Jun 2004 15:51:37 -0000      1.14
+++ java/text/Collator.java     15 Jun 2004 22:37:11 -0000
@@ -303,7 +303,7 @@
     try
       {
        res = ResourceBundle.getBundle("gnu.java.locale.LocaleInformation",
-                                      loc, Collator.class.getClassLoader());
+                                      loc, ClassLoader.getSystemClassLoader());
        pattern = res.getString("collation_rules");
       }
     catch (MissingResourceException x)
Index: java/text/DateFormat.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/DateFormat.java,v
retrieving revision 1.14
diff -u -r1.14 DateFormat.java
--- java/text/DateFormat.java   14 Jun 2004 15:51:37 -0000      1.14
+++ java/text/DateFormat.java   15 Jun 2004 22:37:11 -0000
@@ -325,7 +325,7 @@
     try
       {
        res = ResourceBundle.getBundle("gnu.java.locale.LocaleInformation",
-                                      loc, DateFormat.class.getClassLoader());
+                                      loc, ClassLoader.getSystemClassLoader());
       }
     catch (MissingResourceException x)
       {
Index: java/text/DateFormatSymbols.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/DateFormatSymbols.java,v
retrieving revision 1.16
diff -u -r1.16 DateFormatSymbols.java
--- java/text/DateFormatSymbols.java    14 Jun 2004 15:51:37 -0000      1.16
+++ java/text/DateFormatSymbols.java    15 Jun 2004 22:37:11 -0000
@@ -99,7 +99,7 @@
   {
     ResourceBundle res
       = ResourceBundle.getBundle("gnu.java.locale.LocaleInformation", locale,
-                                getClass().getClassLoader());
+                                ClassLoader.getSystemClassLoader());
 
     ampms = res.getStringArray ("ampms");
     eras = res.getStringArray ("eras");
Index: java/text/DecimalFormatSymbols.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/DecimalFormatSymbols.java,v
retrieving revision 1.14
diff -u -r1.14 DecimalFormatSymbols.java
--- java/text/DecimalFormatSymbols.java 14 Jun 2004 15:51:37 -0000      1.14
+++ java/text/DecimalFormatSymbols.java 15 Jun 2004 22:37:11 -0000
@@ -130,7 +130,7 @@
     try
       {
        res = ResourceBundle.getBundle("gnu.java.locale.LocaleInformation",
-               loc, DecimalFormatSymbols.class.getClassLoader());
+               loc, ClassLoader.getSystemClassLoader());
       }
     catch (MissingResourceException x)
       {
Index: java/text/NumberFormat.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/NumberFormat.java,v
retrieving revision 1.14
diff -u -r1.14 NumberFormat.java
--- java/text/NumberFormat.java 14 Jun 2004 15:51:37 -0000      1.14
+++ java/text/NumberFormat.java 15 Jun 2004 22:37:11 -0000
@@ -310,7 +310,7 @@
     try
       {
        res = ResourceBundle.getBundle("gnu.java.locale.LocaleInformation",
-               loc, NumberFormat.class.getClassLoader());
+               loc, ClassLoader.getSystemClassLoader());
       }
     catch (MissingResourceException x)
       {
Index: java/awt/Window.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/awt/Window.java,v
retrieving revision 1.35
diff -u -r1.35 Window.java
--- java/awt/Window.java        14 Jun 2004 15:51:37 -0000      1.35
+++ java/awt/Window.java        15 Jun 2004 22:37:11 -0000
@@ -704,7 +704,7 @@
   public void applyResourceBundle(String rbName)
   {
     ResourceBundle rb = ResourceBundle.getBundle(rbName, Locale.getDefault(),
-      Window.class.getClassLoader());
+      ClassLoader.getSystemClassLoader());
     if (rb != null)
       applyResourceBundle(rb);    
   }