JBoss ClassLoader
  1. JBoss ClassLoader
  2. JBCL-118

getResources in BaseClassLoader, ClassLoaderDomain, and friends uses Set<URL> with HashSet

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved (View Workflow)
    • Priority: Major Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: JBossCL.2.0.7.CR1
    • Component/s: None
    • Labels:
      None
    • Similar Issues:
      Show 10 results 

      Description

      BaseClassLoader, ClassLoaderDomain and several other classes in JBossCL have public API methods which take Set as a parameter. We can't control what implementation of Set is passed in to these methods, but HashSet is commonly used, causing the URL.hashCode() performance problem (DNS lookup).

      I do not understand enough about the public API of JBossCL to know where these methods might be used from. However, I do know that these methods are called internally from several areas in JBossCL with a HashSet.

      I see a two-step solution:

      1) Change anywhere in JBossCL where HashSet is used to another Set implementation. This is a quick and non-API-breaking change that will actually take care of the most common scenarios.

      An example (from user code in the container): ClassLoader.getResources(String) eventually calls BaseClassLoader.loadResources(String) which creates a HashSet and passes that to ClassLoaderDomain.getResources(...) to be filled in, eventually returning the HashSet to the caller. Changing the internal usage of ClassLoaderDomain and friends to use a Set other than HashSet would fix this scenario.

      2) Consider if there is an API change required to prevent misuse of this by callers outside of JBossCL. I'm not sure if any of these methods are intended to be used from outside. Some are public, some are not.

      I'm willing to tackle solution #1 and may submit a patch later this evening.

      However, I need some help/discussion/advice before I could tackle #2.

      1. JBCL-118.patch
        3 kB
        Tolga Tarhan
      2. vfs-patch-JBCL-118.patch
        0.8 kB
        Tolga Tarhan

        Issue Links

          Activity

          Hide
          Tolga Tarhan
          added a comment -

          A patch which replaces all usages of HashSet<URL> with TreeSet<URL> within JBossCL. Also includes a custom URLComparator since TreeSet requires one. Performance impact of switching Set implementations should be trivial since all of the usage is constrained to getResources(), which doesn't return a very large Set most of the time.

          This doesn't solve the larger issue that other bits of JBoss may use these methods with HashSet.

          Show
          Tolga Tarhan
          added a comment - A patch which replaces all usages of HashSet<URL> with TreeSet<URL> within JBossCL. Also includes a custom URLComparator since TreeSet requires one. Performance impact of switching Set implementations should be trivial since all of the usage is constrained to getResources(), which doesn't return a very large Set most of the time. This doesn't solve the larger issue that other bits of JBoss may use these methods with HashSet.
          Hide
          Tolga Tarhan
          added a comment -

          A snapshot of jboss-classloader.jar which is built with the patch against tags/2.0.6.GA, thus making it a drop-in replacement for JBoss 5.1.0.GA.

          Show
          Tolga Tarhan
          added a comment - A snapshot of jboss-classloader.jar which is built with the patch against tags/2.0.6.GA, thus making it a drop-in replacement for JBoss 5.1.0.GA.
          Hide
          Tolga Tarhan
          added a comment -

          Edit: The previous jboss-classloader.jar attachment was the wrong copy. Here's the corrected one.

          A snapshot of jboss-classloader.jar which is built with the patch against tags/2.0.6.GA, thus making it a drop-in replacement for JBoss 5.1.0.GA.

          Show
          Tolga Tarhan
          added a comment - Edit: The previous jboss-classloader.jar attachment was the wrong copy. Here's the corrected one. A snapshot of jboss-classloader.jar which is built with the patch against tags/2.0.6.GA, thus making it a drop-in replacement for JBoss 5.1.0.GA.
          Hide
          Tolga Tarhan
          added a comment -

          As suggested by Adrian in JBCL-117, a better fix is to modify org.jboss.virtual.protocol.vfsmemory.Handler to override the DNS-resolving behavior of hashCode() and equals(). The attached patch does this by overriding getHostAddress(URL) inside org.jboss.virtual.protocol.vfsmemory.Handler to always return null. This is also semantically correct, since vfsmemory "hosts" are not real hosts at all.

          Show
          Tolga Tarhan
          added a comment - As suggested by Adrian in JBCL-117 , a better fix is to modify org.jboss.virtual.protocol.vfsmemory.Handler to override the DNS-resolving behavior of hashCode() and equals(). The attached patch does this by overriding getHostAddress(URL) inside org.jboss.virtual.protocol.vfsmemory.Handler to always return null. This is also semantically correct, since vfsmemory "hosts" are not real hosts at all.
          Hide
          Tolga Tarhan
          added a comment -

          A snapshot of the fixed jboss-vfs.jar which is compatible with JBoss-5.1.0.GA for anyone who needs it is attached.

          Show
          Tolga Tarhan
          added a comment - A snapshot of the fixed jboss-vfs.jar which is compatible with JBoss-5.1.0.GA for anyone who needs it is attached.
          Hide
          Ales Justin
          added a comment -

          Same as JBCL-117.

          Show
          Ales Justin
          added a comment - Same as JBCL-117 .

            People

            • Assignee:
              Ales Justin
              Reporter:
              Tolga Tarhan
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: