RVM

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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
Andrew John Hughes added a comment -

Filed as a bug in Classpath:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33690

Show
Andrew John Hughes added a comment - Filed as a bug in Classpath: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33690
Hide
Permalink
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
Permalink
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
Permalink
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
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: