Name: mt13159 Date: 10/17/2001


The sun.applet.AppletSecurity.checkPackageAccess() method is extremely inefficient. This impacts applets running in both the plugin and appletviewer in JDK 1.2 and later (and all the way up through the current 1.4 beta releases).
The problem is that this method does a lot of string manipulation and property lookups every time it is called, and it is called for every class that is loaded by the applet. The problem is worse the more deeply nested your package names are, because it loops through each element of the package name. The restricted package names are stored in System properties starting with "package.restrict.access." The rest of the property name is the fully qualified name of the restricted package. When checkPackageAccess() is called, it takes the fully qualified name of the class being defined, chops off the class name to obtain the package name, then does a property lookup to see if there is a "package.restrict.access." property for this package. Then it chops off the last element in the package name and does the same thing again, and so on.
All of this work could be done up front, however, to save a lot of time. You could enumerate all the system properties beginning with "package.restrict.access." and store them in a hash table. Then checkPackageAccess() becomes a simple hash lookup. For example, the following field could be added to the AppletSecurity class:

private HashSet restrictedPackages = new HashSet();

Then, the following code can be added to the AppletSecurity constructor:

// Read in the list of restricted packages from the system properties
AccessController.doPrivileged(new PrivilegedAction(){
  public Object run() {
    Enumeration e = System.getProperties().propertyNames();
    while (e.hasMoreElements()) {
      String name = (String)e.nextElement();
      if (name.startsWith("package.restrict.access.")) {
        String value = System.getProperty(name);
        if (value.equalsIgnoreCase("true")) {
          String pkg = name.substring(24);
          int i;
          do {
            restrictedPackages.add(pkg);
            if ((i = pkg.lastIndexOf('.')) != -1) {
               pkg = pkg.substring(0, i);
            }
          } while (i != -1);
        }
      }
    }
    return null;
  }
});

Then, checkPackageAccess() simply becomes:

public void checkPackageAccess(final String pkgname) {
  // first see if the VM-wide policy allows access to this package
  super.checkPackageAccess(pkgname);

  // now check the list of restricted packages
  if (restrictedPackages.contains(pkgname)) {
    checkPermission(new java.lang.RuntimePermission
                    ("accessClassInPackage." + pkgname));
    }
}

It could be argued that the above code changes would alter the behavior, because now it would take a snapshot of the System properties at the time the AppletSecurity class is created. So any change to the properties made later would never be picked up. However, it doesn't make sense to alter the list of restricted packages once the applet has been started. No provision is made for restricting access to classes that have already been loaded should the list of restricted packages change at a later time. So it does not seem practical to have such a large performance hit to allow a behavior that doesn't really make sense anyway.

This may seem like a minor inefficiency, but we have found that it is actual a critical bottleneck for us. Some of our applications load thousands of classes. Our average package depth is 3 or 4, so every time one of these classes is loaded, numerous string manipulations and property lookups occur. Testing and profiling has shown that 10%-20% of startup time is spent in the checkPackageAccess() method for some of our larger applications. This time is eliminated with the above code changes.
(Review ID: 133786)
======================================================================