[cp-patches] ResourceBundle

[Bryce McKinlay]

This patch merges in updated ResourceBundle code from libgcj. In 
particular, it:

- Significantly simplifies the bundle lookup code, which should now be 
much more efficient & maintainable.
- Changes the bundle caching strategy for improved look-up efficiency 
(there are now no allocations for lookup in the common case)

For more details/explanation, see here:

http://gcc.gnu.org/ml/java-patches/2004-q3/msg00135.html

This patch differs slightly from the libgcj version because libgcj has a 
native "getCallingClassLoader" while classpath implements this in Java. 
Everything else is fully merged, however.

We've been using this in libgcj for many months now without problems, so 
I'm pretty confident that its stable. However, Mark asked me to post the 
patch here before checking it in, so here it is.

Bryce


2004-11-16  Tom Tromey  <address@hidden>

        * java/util/ResourceBundle.java (tryBundle): Use
        Class.isAssignableFrom rather than catching ClassCastException.

2004-11-16  Bryce McKinlay  <address@hidden>

        * java/util/ResourceBundle.java (bundleCache): Renamed from
        resourceBundleCache. Update comments.
        (getObject): Don't catch MissingResourceException.
        (getBundle(String)): Remove 'final'. Use system classloader if
        getCallingClassLoader returned null.
        (getBundle(String, Locale)): Likewise.
        (BundleKey): New private class. HashMap key for bundle cache lookup.
        (lookupKey): New. Singleton instance of BundleKey.
        (nullEntry): New. Cache entry to represent failed lookups.
        (getBundle(String, Locale, ClassLoader)): Re-written to use new 
        caching strategy, no-allocation lookup, and new tryBundle methods.
        (tryBundle(String, ClassLoader)): New. Load a locale-qualified bundle 
        name using given classloader.
        (tryBundle(String, Locale, ClassLoader, boolean): New. Qualify 
        baseName for given Locale and attempt to load bundle.

Index: java/util/ResourceBundle.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/ResourceBundle.java,v
retrieving revision 1.25
diff -u -r1.25 ResourceBundle.java
--- java/util/ResourceBundle.java       22 Oct 2004 18:02:06 -0000      1.25
+++ java/util/ResourceBundle.java       17 Nov 2004 01:21:21 -0000
@@ -104,7 +104,7 @@
    * <code>getBundle</code>.
    */
   private Locale locale;
-      
+
   /**
    * We override SecurityManager in order to access getClassContext().
    */
@@ -125,10 +125,10 @@
     {
       Class[] stack = getClassContext();
       for (int i = 0; i < stack.length; i++)
-       {
-         if (stack[i] != Security.class && stack[i] != ResourceBundle.class)
-           return stack[i].getClassLoader();
-       }
+       {
+        if (stack[i] != Security.class && stack[i] != ResourceBundle.class)
+          return stack[i].getClassLoader();
+       }
 
       return null;
     }
@@ -138,22 +138,19 @@
   private static final Security security
     = (Security) AccessController.doPrivileged(new PrivilegedAction()
       {
-        // This will always work since java.util classes have (all) system
-        // permissions.
-        public Object run()
-        {
-          return new Security();
-        }
+       // This will always work since java.util classes have (all) system
+       // permissions.
+       public Object run()
+       {
+         return new Security();
+       }
       }
     );
 
   /**
-   * The resource bundle cache. This is a two-level hash map: The key
-   * is the class loader, the value is a new HashMap. The key of this
-   * second hash map is the localized name, the value is a soft
-   * references to the resource bundle.
+   * The resource bundle cache.
    */
-  private static Map resourceBundleCache;
+  private static Map bundleCache;
 
   /**
    * The last default Locale we saw. If this ever changes then we have to
@@ -215,15 +212,11 @@
   public final Object getObject(String key)
   {
     for (ResourceBundle bundle = this; bundle != null; bundle = bundle.parent)
-      try
-        {
-          Object o = bundle.handleGetObject(key);
-          if (o != null)
-            return o;
-        }
-      catch (MissingResourceException ex)
-        {
-        }
+      {
+        Object o = bundle.handleGetObject(key);
+        if (o != null)
+          return o;
+      }
  
     throw new MissingResourceException("Key not found", getClass().getName(),
                                       key);
@@ -263,10 +256,12 @@
    * @throws MissingResourceException if the resource bundle can't be found
    * @throws NullPointerException if baseName is null
    */
-  public static final ResourceBundle getBundle(String baseName)
+  public static ResourceBundle getBundle(String baseName)
   {
-    return getBundle(baseName, Locale.getDefault(),
-                     security.getCallingClassLoader());
+    ClassLoader cl = security.getCallingClassLoader();
+    if (cl == null)
+      cl = ClassLoader.getSystemClassLoader();
+    return getBundle(baseName, Locale.getDefault(), cl);
   }
 
   /**
@@ -281,12 +276,63 @@
    * @throws MissingResourceException if the resource bundle can't be found
    * @throws NullPointerException if baseName or locale is null
    */
-  public static final ResourceBundle getBundle(String baseName,
-                                               Locale locale)
+  public static ResourceBundle getBundle(String baseName, Locale locale)
   {
-    return getBundle(baseName, locale, security.getCallingClassLoader());
+    ClassLoader cl = security.getCallingClassLoader();
+    if (cl == null)
+      cl = ClassLoader.getSystemClassLoader();
+    return getBundle(baseName, locale, cl);
   }
 
+  /** Cache key for the ResourceBundle cache.  Resource bundles are keyed
+      by the combination of bundle name, locale, and class loader. */
+  private static class BundleKey
+  {
+    String baseName;
+    Locale locale;
+    ClassLoader classLoader;
+    int hashcode;
+
+    BundleKey() {}
+
+    BundleKey(String s, Locale l, ClassLoader cl)
+    {
+      set(s, l, cl);
+    }
+    
+    void set(String s, Locale l, ClassLoader cl)
+    {
+      baseName = s;
+      locale = l;
+      classLoader = cl;
+      hashcode = baseName.hashCode() ^ locale.hashCode() ^
+        classLoader.hashCode();
+    }
+    
+    public int hashCode()
+    {
+      return hashcode;
+    }
+    
+    public boolean equals(Object o)
+    {
+      if (! (o instanceof BundleKey))
+        return false;
+      BundleKey key = (BundleKey) o;
+      return hashcode == key.hashcode &&
+       baseName.equals(key.baseName) &&
+        locale.equals(key.locale) &&
+       classLoader.equals(key.classLoader);
+    }    
+  }
+  
+  /** A cache lookup key. This avoids having to a new one for every
+   *  getBundle() call. */
+  private static BundleKey lookupKey = new BundleKey();
+  
+  /** Singleton cache entry to represent previous failed lookups. */
+  private static Object nullEntry = new Object();
+
   /**
    * Get the appropriate ResourceBundle for the given locale. The following
    * strategy is used:
@@ -363,81 +409,60 @@
    */
   // This method is synchronized so that the cache is properly
   // handled.
-  public static final synchronized ResourceBundle getBundle
+  public static synchronized ResourceBundle getBundle
     (String baseName, Locale locale, ClassLoader classLoader)
   {
-    // This implementation searches the bundle in the reverse direction
-    // and builds the parent chain on the fly.
+    // If the default locale changed since the last time we were called,
+    // all cache entries are invalidated.
     Locale defaultLocale = Locale.getDefault();
     if (defaultLocale != lastDefaultLocale)
       {
-       resourceBundleCache = new HashMap();
+       bundleCache = new HashMap();
        lastDefaultLocale = defaultLocale;
       }
-    HashMap cache = (HashMap) resourceBundleCache.get(classLoader);
-    StringBuffer sb = new StringBuffer(60);
-    sb.append(baseName).append('_').append(locale);
-    String name = sb.toString();
 
-    if (cache == null)
+    // This will throw NullPointerException if any arguments are null.
+    lookupKey.set(baseName, locale, classLoader);
+    
+    Object obj = bundleCache.get(lookupKey);
+    ResourceBundle rb = null;
+    
+    if (obj instanceof ResourceBundle)
       {
-        cache = new HashMap();
-        resourceBundleCache.put(classLoader, cache);
+        return (ResourceBundle) obj;
       }
-    else if (cache.containsKey(name))
+    else if (obj == nullEntry)
       {
-       Reference ref = (Reference) cache.get(name);
-       // If REF is null, that means that we added a `null' value to
-       // the hash map.  That means we failed to find the bundle
-       // previously, and we cached that fact.  The JDK does this, so
-       // it must be ok.
-       if (ref == null)
-         throw new MissingResourceException("Bundle " + baseName
-                                            + " not found",
-                                            baseName, "");
-       else
-         {
-           ResourceBundle rb = (ResourceBundle) ref.get();
-           if (rb != null)
-             {
-               // RB should already have the right parent, except if
-               // something very strange happened.
-               return rb;
-             }
-           // If RB is null, then we previously found it but it was
-           // collected.  So we try again.
-         }
+        // Lookup has failed previously. Fall through.
       }
-
-    // It is ok if this returns null.  We aren't required to have the
-    // base bundle.
-    ResourceBundle baseBundle = tryBundle(baseName, emptyLocale,
-                                          classLoader, null, cache);
-
-    // Now use our locale, followed by the default locale.  We only
-    // need to try the default locale if our locale is different, and
-    // if our locale failed to yield a result other than the base
-    // bundle.
-    ResourceBundle bundle = tryLocalBundle(baseName, locale,
-                                           classLoader, baseBundle, cache);
-    if (bundle == baseBundle && !locale.equals(defaultLocale))
+    else
       {
-       bundle = tryLocalBundle(baseName, defaultLocale,
-                               classLoader, baseBundle, cache);
-       // We need to record that the argument locale maps to the
-       // bundle we just found.  If we didn't find a bundle, record
-       // that instead.
-       if (bundle == null)
-         cache.put(name, null);
+       // First, look for a bundle for the specified locale. We don't want
+       // the base bundle this time.
+       boolean wantBase = locale.equals(defaultLocale);
+       ResourceBundle bundle = tryBundle(baseName, locale, classLoader, 
+                                         wantBase);
+
+        // Try the default locale if neccessary.
+       if (bundle == null && !locale.equals(defaultLocale))
+         bundle = tryBundle(baseName, defaultLocale, classLoader, true);
+
+       BundleKey key = new BundleKey(baseName, locale, classLoader);
+        if (bundle == null)
+         {
+           // Cache the fact that this lookup has previously failed.
+           bundleCache.put(key, nullEntry);
+         }
        else
-         cache.put(name, new SoftReference(bundle));
+         {
+            // Cache the result and return it.
+           bundleCache.put(key, bundle);
+           return bundle;
+         }
       }
 
-    if (bundle == null)
-      throw new MissingResourceException("Bundle " + baseName + " not found",
-                                        baseName, "");
-
-    return bundle;
+    throw new MissingResourceException("Bundle " + baseName + " not found",
+                                      baseName, "");
   }
 
   /**
@@ -466,42 +491,13 @@
    * Tries to load a class or a property file with the specified name.
    *
    * @param localizedName the name
-   * @param locale the locale, that must be used exactly
    * @param classloader the classloader
-   * @param bundle the backup (parent) bundle
    * @return the resource bundle if it was loaded, otherwise the backup
    */
-  private static ResourceBundle tryBundle(String localizedName, Locale locale,
-                                          ClassLoader classloader,
-                                          ResourceBundle bundle, HashMap cache)
+  private static ResourceBundle tryBundle(String localizedName,
+                                          ClassLoader classloader)
   {
-    // First look into the cache.
-    if (cache.containsKey(localizedName))
-      {
-       Reference ref = (Reference) cache.get(localizedName);
-       // If REF is null, that means that we added a `null' value to
-       // the hash map.  That means we failed to find the bundle
-       // previously, and we cached that fact.  The JDK does this, so
-       // it must be ok.
-       if (ref == null)
-         return null;
-       else
-         {
-           ResourceBundle rb = (ResourceBundle) ref.get();
-           if (rb != null)
-             {
-               // RB should already have the right parent, except if
-               // something very strange happened.
-               return rb;
-             }
-           // If RB is null, then we previously found it but it was
-           // collected.  So we try again.
-         }
-      }
-
-    // foundBundle holds exact matches for the localizedName resource
-    // bundle, which may later be cached.
-    ResourceBundle foundBundle = null;
+    ResourceBundle bundle = null;
     try
       {
         Class rbClass;
@@ -509,98 +505,118 @@
           rbClass = Class.forName(localizedName);
         else
           rbClass = classloader.loadClass(localizedName);
-        foundBundle = (ResourceBundle) rbClass.newInstance();
-        foundBundle.parent = bundle;
-        foundBundle.locale = locale;
-      }
-    catch (Exception ex)
-      {
-        // ignore them all
-       foundBundle = null;
-      }
-    if (foundBundle == null)
+       // Note that we do the check up front instead of catching
+       // ClassCastException.  The reason for this is that some crazy
+       // programs (Eclipse) have classes that do not extend
+       // ResourceBundle but that have the same name as a property
+       // bundle; in fact Eclipse relies on ResourceBundle not
+       // instantiating these classes.
+       if (ResourceBundle.class.isAssignableFrom(rbClass))
+         bundle = (ResourceBundle) rbClass.newInstance();
+      }
+    catch (IllegalAccessException ex) {}
+    catch (InstantiationException ex) {}
+    catch (ClassNotFoundException ex) {}
+
+    if (bundle == null)
       {
        try
          {
            InputStream is;
-           final String resourceName
+           String resourceName
              = localizedName.replace('.', '/') + ".properties";
            if (classloader == null)
              is = ClassLoader.getSystemResourceAsStream(resourceName);
            else
              is = classloader.getResourceAsStream(resourceName);
            if (is != null)
-             {
-               foundBundle = new PropertyResourceBundle(is);
-               foundBundle.parent = bundle;
-               foundBundle.locale = locale;
-             }
+             bundle = new PropertyResourceBundle(is);
          }
        catch (IOException ex)
          {
+           MissingResourceException mre = new MissingResourceException
+             ("Failed to load bundle", localizedName, "");
+           mre.initCause(ex);
+           throw mre;
          }
       }
 
-    // Put the result into the hash table.  If we didn't find anything
-    // here, we record our parent bundle.  If we record `null' that means
-    // nothing, not even the base, was found.
-    if (foundBundle == null)
-      foundBundle = bundle;
-    if (foundBundle == null)
-      cache.put(localizedName, null);
-    else
-      cache.put(localizedName, new SoftReference(foundBundle));
-    return foundBundle;
+    return bundle;
   }
 
   /**
    * Tries to load a the bundle for a given locale, also loads the backup
    * locales with the same language.
    *
-   * @param name the name
+   * @param baseName the raw bundle name, without locale qualifiers
    * @param locale the locale
    * @param classloader the classloader
    * @param bundle the backup (parent) bundle
+   * @param wantBase whether a resource bundle made only from the base name
+   *        (with no locale information attached) should be returned.
    * @return the resource bundle if it was loaded, otherwise the backup
    */
-  private static ResourceBundle tryLocalBundle(String baseName, Locale locale,
-                                               ClassLoader classloader,
-                                               ResourceBundle bundle,
-                                               HashMap cache)
-  {
-    final String language = locale.getLanguage();
-    final String country = locale.getCountry();
-    final String variant = locale.getVariant();
+  private static ResourceBundle tryBundle(String baseName, Locale locale,
+                                          ClassLoader classLoader, 
+                                         boolean wantBase)
+  {
+    String language = locale.getLanguage();
+    String country = locale.getCountry();
+    String variant = locale.getVariant();
+    
+    int baseLen = baseName.length();
+
+    // Build up a StringBuffer containing the complete bundle name, fully
+    // qualified by locale.
+    StringBuffer sb = new StringBuffer(baseLen + variant.length() + 7);
 
-    StringBuffer sb = new StringBuffer(60);
     sb.append(baseName);
-    sb.append('_');
-
+    
     if (language.length() > 0)
       {
+       sb.append('_');
        sb.append(language);
-       bundle = tryBundle(sb.toString(), new Locale(language),
-                          classloader, bundle, cache);
-      }
-    // If LANGUAGE was empty, we still need to try the other
-    // components, and the `_' is required.
-    sb.append('_');
-
-    if (country.length() > 0)
-      {
-       sb.append(country);
-       bundle = tryBundle(sb.toString(), new Locale(language, country),
-                          classloader, bundle, cache);
+       
+       if (country.length() > 0)
+         {
+           sb.append('_');
+           sb.append(country);
+           
+           if (variant.length() > 0)
+             {
+               sb.append('_');
+               sb.append(variant);
+             }
+         }
       }
-    sb.append('_');
 
-    if (variant.length() > 0)
+    // Now try to load bundles, starting with the most specialized name.
+    // Build up the parent chain as we go.
+    String bundleName = sb.toString();
+    ResourceBundle first = null; // The most specialized bundle.
+    ResourceBundle last = null; // The least specialized bundle.
+    
+    while (true)
       {
-       sb.append(variant);
-       bundle = tryBundle(sb.toString(), locale,
-                          classloader, bundle, cache);
+        ResourceBundle foundBundle = tryBundle(bundleName, classLoader);
+       if (foundBundle != null)
+         {
+           if (first == null)
+             first = foundBundle;
+           if (last != null)
+             last.parent = foundBundle;
+           foundBundle.locale = locale;
+           last = foundBundle;
+         }
+       int idx = bundleName.lastIndexOf('_');
+       // Try the non-localized base name only if we already have a
+       // localized child bundle, or wantBase is true.
+       if (idx > baseLen || (idx == baseLen && (first != null || wantBase)))
+         bundleName = bundleName.substring(0, idx);
+       else
+         break;
       }
-
-    return bundle;
+    
+    return first;
   }
 }