RVM
  1. RVM
  2. RVM-185

Reduce cost of ThreadLocal(s) to improve Jython performance

    Details

    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

      Classpath's implementation of ThreadLocal(s) is expensive for get operations. The DaCapo Jython benchmark holds the state of the runtime in a thread local and uses it frequently (nearly every python operator) to make decisions, amongst other things, about adaptive compilation. The Classpath implementation of ThreadLocal(s) is to use a WeakHashMap, so every ThreadLocal get turns into a WeakHashMap get, which makes the cost of operators in python considerably more expensive. We should switch from using a WeakHashMap for ThreadLocal variables to using a simpler data structure such as an array.

        Activity

        Hide
        Ian Rogers added a comment -

        This SVN diff gives the changes necessary to build my revised ThreadLocal with the Jikes RVM.

        Show
        Ian Rogers added a comment - This SVN diff gives the changes necessary to build my revised ThreadLocal with the Jikes RVM.
        Hide
        Ian Rogers added a comment -

        Daniel is experimenting with a revised version of ThreadLocal that uses a cheaper weak hash map. I'm not committing the changes I made for this issue and instead letting Daniel compare my patch with his own, prior to one or other making it into the system and/or Classpath.

        Show
        Ian Rogers added a comment - Daniel is experimenting with a revised version of ThreadLocal that uses a cheaper weak hash map. I'm not committing the changes I made for this issue and instead letting Daniel compare my patch with his own, prior to one or other making it into the system and/or Classpath.
        Hide
        Daniel Frampton added a comment -

        Changed to a fast ThreadLocalMap implementation in 13365.

        Can revisit in the future if this becomes a bottleneck again.

        Show
        Daniel Frampton added a comment - Changed to a fast ThreadLocalMap implementation in 13365. Can revisit in the future if this becomes a bottleneck again.
        Hide
        Andrew John Hughes added a comment -
        Show
        Andrew John Hughes added a comment - Filed as a bug in Classpath: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33690
        Hide
        Andrew John Hughes added a comment -

        We need to open the discussion on this again on the Classpath lists.

        This code:

        • Map<ThreadLocal<T>,T> map = (Map<ThreadLocal<T>,T>) Thread.getThreadLocals();
          + ThreadLocalMap map = Thread.getThreadLocals();
          // Note that we don't have to synchronize, as only this thread will
          // ever modify the map.
        • T value = map.get(this);
        • if (value == null)
          + T value = (T) map.get(this);
          + if (value == notFound)

        worries me a bit as it seems to be making casts that could possibly be avoided. Is there any way to add the necessary information to ThreadLocalMap?

        Show
        Andrew John Hughes added a comment - We need to open the discussion on this again on the Classpath lists. This code: Map<ThreadLocal<T>,T> map = (Map<ThreadLocal<T>,T>) Thread.getThreadLocals(); + ThreadLocalMap map = Thread.getThreadLocals(); // Note that we don't have to synchronize, as only this thread will // ever modify the map. T value = map.get(this); if (value == null) + T value = (T) map.get(this); + if (value == notFound) worries me a bit as it seems to be making casts that could possibly be avoided. Is there any way to add the necessary information to ThreadLocalMap?
        Hide
        Ian Rogers added a comment -

        Has there been any progress made on this? The patch still isn't in Classpath's repository, is Daniel's Classpath paper work in place and have the small cast related tweaks been made?

        Show
        Ian Rogers added a comment - Has there been any progress made on this? The patch still isn't in Classpath's repository, is Daniel's Classpath paper work in place and have the small cast related tweaks been made?
        Hide
        Andrew John Hughes added a comment -

        Patch now in GNU Classpath.

        Show
        Andrew John Hughes added a comment - Patch now in GNU Classpath.

          People

          • Assignee:
            Unassigned
            Reporter:
            Ian Rogers
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: