Uploaded image for project: 'JikesRVM'
  1. JikesRVM
  2. RVM-185

Reduce cost of ThreadLocal(s) to improve Jython performance

    Details

      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.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            ianrogers Ian Rogers added a comment -

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

            Show
            ianrogers Ian Rogers added a comment - This SVN diff gives the changes necessary to build my revised ThreadLocal with the Jikes RVM.
            Hide
            ianrogers 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
            ianrogers 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
            zyridium 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
            zyridium 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
            gnu_andrew Andrew John Hughes added a comment -
            Show
            gnu_andrew Andrew John Hughes added a comment - Filed as a bug in Classpath: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33690
            Hide
            gnu_andrew 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
            gnu_andrew 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
            ianrogers 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
            ianrogers 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
            gnu_andrew Andrew John Hughes added a comment -

            Patch now in GNU Classpath.

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

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: