• New Feature
  • Status: Resolved
  • 2 Major
  • Resolution: Won't Fix
  • Byte Code Transform
  • interfaces
  • Reporter: amiller
  • April 24, 2008
  • 1
  • Watchers: 1
  • February 12, 2014
  • February 12, 2014

Description

Currently, we modify ReentrantLock to force the use of fair lock policy as our instrumented version extends from the fair lock object. This sucks as the performance of instrumented unclustered locks will be significantly impacted by this change (I’ve seen 100x difference in microbenchmarks I wrote for other purposes).

In code review today we kicked around some ideas for fixing this. Since we don’t allow a lock to become clustered while it’s locked, we should be guaranteed that no one is locking the lock and may be able to make this switch at the time it becomes clustered instead of at construction time. Another idea was to actually not extend from the FairSync for our own support since we kind of supersede the behavior in our own version anyway.

Comments

David Connard 2010-08-25

In case other devs stumble across this JIRA ticket while investigating why their ReentrantLocks are running in fair mode when it wasn’t requested, the following workaround was successful for me in reverting TC’s global instrumentation changes:

boolean useFairLocks = …; ReentrantLock lock = new ReentrantLock(useFairLocks); if (!useFairLocks && lock.isFair()) { // There is a problem in terracotta that forces “fair locks” to be used regardless of the requested setting // and this can come with a significant performance penalty under heavy load // // see: // https://jira.terracotta.org/jira/browse/CDV-724 // https://jira.terracotta.org/jira/browse/CDV-1434 // logger.log(Level.INFO, “Found ReentrantLock to be fair when it should not have been, attempting to unset”); Field f = null; Constructor ctr = null; try { f = lock.getClass().getDeclaredField(“sync”); Class c = Class.forName(“java.util.concurrent.locks.ReentrantLock$NonfairSync”); ctr = c.getDeclaredConstructor(); ctr.setAccessible(true); Object sync = ctr.newInstance(); f.setAccessible(true); f.set(lock, sync); } catch (Exception ex) { logger.log(Level.WARNING, “Failed to unset fair status of ReentrantLock”, ex); } finally { // clean up… if (f != null) f.setAccessible(false); if (ctr != null) ctr.setAccessible(false); } }

Yes, it’s ugly and somewhat brittle, and yes, I’d rather not have it in my code… but while this problem exists it’s necessary.

It’s also worth pointing out that many of the new Java 1.6 concurrent BlockingQueue implementations (eg. ArrayBlockingQueue, LinkedBlockingQueue), use ReentrantLocks internally and that performance of these will suffer under TC as a result. Ditto for ConcurrentHashMap. Avoiding these new java.util.concurrent classes and sticking with ye-olde queue implementations via synchronized blocks if you’re running under TC may prove worthwhile until this is fixed.

Hung Huynh 2014-02-12

DSO is discontinued