You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2009/12/09 04:17:16 UTC

svn commit: r888688 - in /tapestry/tapestry5/trunk/tapestry-ioc/src: main/java/org/apache/tapestry5/ioc/internal/services/ main/java/org/apache/tapestry5/ioc/internal/util/ test/java/org/apache/tapestry5/ioc/internal/util/

Author: hlship
Date: Wed Dec  9 03:17:14 2009
New Revision: 888688

URL: http://svn.apache.org/viewvc?rev=888688&view=rev
Log:
TAP5-945: Unnecessary and severe lock contention in PerthreadManagerImpl

Added:
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/DummyLock.java   (with props)
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/JDKUtils.java   (with props)
    tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/DummyLockTest.java   (with props)
Modified:
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java?rev=888688&r1=888687&r2=888688&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java Wed Dec  9 03:17:14 2009
@@ -14,18 +14,24 @@
 
 package org.apache.tapestry5.ioc.internal.services;
 
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
+import org.apache.tapestry5.ioc.internal.util.DummyLock;
+import org.apache.tapestry5.ioc.internal.util.JDKUtils;
 import org.apache.tapestry5.ioc.services.PerthreadManager;
 import org.apache.tapestry5.ioc.services.ThreadCleanupListener;
 import org.slf4j.Logger;
 
-import java.util.List;
-import java.util.Map;
-
 public class PerthreadManagerImpl implements PerthreadManager
 {
     private static final String LISTENERS_KEY = "PerthreadManager.listenerList";
 
+    private final Lock lock;
+
     private static class MapHolder extends ThreadLocal<Map>
     {
         @Override
@@ -41,15 +47,29 @@
 
     public PerthreadManagerImpl(Logger logger)
     {
-        this.logger = logger;
+        this(logger, JDKUtils.JDK_1_5);
     }
 
-
-    private synchronized Map getPerthreadMap()
+    PerthreadManagerImpl(Logger logger, boolean useSynchronization)
     {
-        return holder.get();
+        this.logger = logger;
+
+        lock = useSynchronization ? new ReentrantLock() : new DummyLock();
     }
 
+    private Map getPerthreadMap()
+    {
+        try
+        {
+            lock.lock();
+            
+            return holder.get();
+        }
+        finally
+        {
+            lock.unlock();
+        }
+    }
 
     private List<ThreadCleanupListener> getListeners()
     {
@@ -64,14 +84,14 @@
         return result;
     }
 
-
     public void addThreadCleanupListener(ThreadCleanupListener listener)
     {
         getListeners().add(listener);
     }
 
     /**
-     * Instructs the hub to notify all its listeners (for the current thread). It also discards its list of listeners.
+     * Instructs the hub to notify all its listeners (for the current thread).
+     * It also discards its list of listeners.
      */
     public void cleanup()
     {
@@ -91,13 +111,20 @@
             }
         }
 
-        // Listeners should not re-add themselves or store any per-thread state here,
+        // Listeners should not re-add themselves or store any per-thread state
+        // here,
         // it will be lost.
 
-        synchronized (this)
+        try
         {
+            lock.lock();
+            
             holder.remove();
         }
+        finally
+        {
+            lock.unlock();
+        }
     }
 
     public void put(Object key, Object value)

Added: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/DummyLock.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/DummyLock.java?rev=888688&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/DummyLock.java (added)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/DummyLock.java Wed Dec  9 03:17:14 2009
@@ -0,0 +1,58 @@
+// Copyright 2009 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.ioc.internal.util;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.Lock;
+
+/**
+ * Acts like a Lock but all operations are no-ops.
+ */
+public class DummyLock implements Lock
+{
+    public void lock()
+    {
+    }
+
+    public void lockInterruptibly() throws InterruptedException
+    {
+    }
+
+    /**
+     * Returns null.
+     */
+    public Condition newCondition()
+    {
+        return null;
+    }
+
+    /** @return true */
+    public boolean tryLock()
+    {
+        return true;
+    }
+
+    /** @return true */
+    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException
+    {
+        return true;
+    }
+
+    public void unlock()
+    {
+    }
+
+}

Propchange: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/DummyLock.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/JDKUtils.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/JDKUtils.java?rev=888688&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/JDKUtils.java (added)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/JDKUtils.java Wed Dec  9 03:17:14 2009
@@ -0,0 +1,32 @@
+// Copyright 2009 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.ioc.internal.util;
+
+/**
+ * Internal utilities for identifying the JDK version, used in the rare cases
+ * that we are patching around JDK bugs.
+ */
+public class JDKUtils
+{
+    /**
+     * Is the running JVM JDK 1.5?
+     */
+    public static final boolean JDK_1_5 = isVersion("1.5");
+
+    private static boolean isVersion(String versionId)
+    {
+        return System.getProperty("java.specification.version").equals(versionId);
+    }
+}

Propchange: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/JDKUtils.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/DummyLockTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/DummyLockTest.java?rev=888688&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/DummyLockTest.java (added)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/DummyLockTest.java Wed Dec  9 03:17:14 2009
@@ -0,0 +1,37 @@
+// Copyright 2009 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.ioc.internal.util;
+
+import java.util.concurrent.locks.Lock;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class DummyLockTest extends Assert
+{
+    @Test
+    public void dummy_lock_functions_are_noops() throws Exception
+    {
+        Lock lock = new DummyLock();
+
+        lock.lock();
+        lock.unlock();
+        lock.lockInterruptibly();
+
+        assertNull(lock.newCondition());
+        assertTrue(lock.tryLock());
+        assertTrue(lock.tryLock(0, null));
+    }
+}

Propchange: tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/DummyLockTest.java
------------------------------------------------------------------------------
    svn:eol-style = native