Author: rgoers Date: Tue Nov 23 02:27:44 2010 New Revision: 1037973 URL: http://svn.apache.org/viewvc?rev=1037973&view=rev Log: Replace synchronization with locking and/or concurrent objects. Add putFileIfAbsent method to allow callers to safely add to the cache
Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java Tue Nov 23 02:27:44 2010 @@ -18,7 +18,7 @@ package org.apache.commons.vfs2; /** - * The fileCache interface. + * The fileCache interface. Implementations of this interface are expected to be thread safe. * * @author <a href="http://commons.apache.org/vfs/team-list.html">Commons VFS team</a> * @version $Revision$ $Date$ @@ -33,6 +33,14 @@ public interface FilesCache void putFile(final FileObject file); /** + * add a fileobject to the cache if it isn't already present. + * + * @param file the file + * @return true if the file was stored, false otherwise. + */ + boolean putFileIfAbsent(final FileObject file); + + /** * retrieve a file from the cache by its name. * * @param filesystem The FileSystem. Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java Tue Nov 23 02:27:44 2010 @@ -45,6 +45,12 @@ public class DefaultFilesCache extends A files.put(file.getName(), file); } + public boolean putFileIfAbsent(final FileObject file) + { + ConcurrentMap<FileName, FileObject> files = getOrCreateFilesystemCache(file.getFileSystem()); + return files.putIfAbsent(file.getName(), file) == null; + } + public FileObject getFile(final FileSystem filesystem, final FileName name) { Map<FileName, FileObject> files = getOrCreateFilesystemCache(filesystem); Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java Tue Nov 23 02:27:44 2010 @@ -16,8 +16,12 @@ */ package org.apache.commons.vfs2.cache; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.commons.collections.map.AbstractLinkedMap; import org.apache.commons.collections.map.LRUMap; @@ -47,12 +51,16 @@ public class LRUFilesCache extends Abstr private final Log log = LogFactory.getLog(LRUFilesCache.class); /** The FileSystem cache */ - private final Map<FileSystem, Map<FileName, FileObject>> filesystemCache = - new HashMap<FileSystem, Map<FileName, FileObject>>(10); + private final ConcurrentMap<FileSystem, Map<FileName, FileObject>> filesystemCache = + new ConcurrentHashMap<FileSystem, Map<FileName, FileObject>>(10); /** The size of the cache */ private final int lruSize; + private ReadWriteLock rwLock = new ReentrantReadWriteLock(); + private Lock readLock = rwLock.readLock(); + private Lock writeLock = rwLock.writeLock(); + /** * The file cache */ @@ -132,40 +140,81 @@ public class LRUFilesCache extends Abstr public void putFile(final FileObject file) { - synchronized (this) - { - Map<FileName, FileObject> files = getOrCreateFilesystemCache(file.getFileSystem()); + Map<FileName, FileObject> files = getOrCreateFilesystemCache(file.getFileSystem()); + writeLock.lock(); + try + { // System.err.println(">>> " + files.size() + " put:" + file.toString()); files.put(file.getName(), file); } + finally + { + writeLock.unlock(); + } } - public FileObject getFile(final FileSystem filesystem, final FileName name) + + public boolean putFileIfAbsent(final FileObject file) { - synchronized (this) + Map<FileName, FileObject> files = getOrCreateFilesystemCache(file.getFileSystem()); + + writeLock.lock(); + try { - Map<FileName, FileObject> files = getOrCreateFilesystemCache(filesystem); + FileName name = file.getName(); + // System.err.println(">>> " + files.size() + " put:" + file.toString()); + if (files.containsKey(name)) + { + return false; + } + + files.put(name, file); + return true; + } + finally + { + writeLock.unlock(); + } + } + + public FileObject getFile(final FileSystem filesystem, final FileName name) + { + Map<FileName, FileObject> files = getOrCreateFilesystemCache(filesystem); + + readLock.lock(); + try + { // FileObject fo = (FileObject) files.get(name); // System.err.println(">>> " + files.size() + " get:" + name.toString() + " " + fo); return files.get(name); } + finally + { + readLock.unlock(); + } } public void clear(final FileSystem filesystem) { - synchronized (this) + Map<FileName, FileObject> files = getOrCreateFilesystemCache(filesystem); + + writeLock.lock(); + try { // System.err.println(">>> clear fs " + filesystem); - Map<FileName, FileObject> files = getOrCreateFilesystemCache(filesystem); files.clear(); filesystemCache.remove(filesystem); } + finally + { + writeLock.unlock(); + } } @SuppressWarnings("unchecked") // (1) @@ -176,8 +225,8 @@ public class LRUFilesCache extends Abstr { // System.err.println(">>> create fs " + filesystem); - files = new MyLRUMap(filesystem, lruSize); // 1 - filesystemCache.put(filesystem, files); + files = new MyLRUMap(filesystem, lruSize); + filesystemCache.putIfAbsent(filesystem, files); } return files; @@ -188,20 +237,18 @@ public class LRUFilesCache extends Abstr { super.close(); - synchronized (this) - { - // System.err.println(">>> clear all"); + // System.err.println(">>> clear all"); - filesystemCache.clear(); - } + filesystemCache.clear(); } public void removeFile(final FileSystem filesystem, final FileName name) { - synchronized (this) - { - Map<?, ?> files = getOrCreateFilesystemCache(filesystem); + Map<?, ?> files = getOrCreateFilesystemCache(filesystem); + writeLock.lock(); + try + { // System.err.println(">>> " + files.size() + " remove:" + name.toString()); files.remove(name); @@ -211,6 +258,10 @@ public class LRUFilesCache extends Abstr filesystemCache.remove(filesystem); } } + finally + { + writeLock.unlock(); + } } public void touchFile(final FileObject file) Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java Tue Nov 23 02:27:44 2010 @@ -40,6 +40,11 @@ public class NullFilesCache extends Abst { } + public boolean putFileIfAbsent(final FileObject file) + { + return false; + } + public FileObject getFile(final FileSystem filesystem, final FileName name) { return null; Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java Tue Nov 23 02:27:44 2010 @@ -31,6 +31,11 @@ import java.lang.ref.SoftReference; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * This implementation caches every file as long as it is strongly reachable by @@ -50,13 +55,16 @@ public class SoftRefFilesCache extends A */ private final Log log = LogFactory.getLog(SoftRefFilesCache.class); - private final Map<FileSystem, Map<FileName, Reference<FileObject>>> filesystemCache = - new HashMap<FileSystem, Map<FileName, Reference<FileObject>>>(); + private final ConcurrentMap<FileSystem, Map<FileName, Reference<FileObject>>> filesystemCache = + new ConcurrentHashMap<FileSystem, Map<FileName, Reference<FileObject>>>(); private final Map<Reference<FileObject>, FileSystemAndNameKey> refReverseMap = new HashMap<Reference<FileObject>, FileSystemAndNameKey>(100); private final ReferenceQueue<FileObject> refqueue = new ReferenceQueue<FileObject>(); - private SoftRefReleaseThread softRefReleaseThread; + private AtomicReference<SoftRefReleaseThread> softRefReleaseThread = new AtomicReference<SoftRefReleaseThread>(); + + private Lock lock = new ReentrantLock(); + /** * This thread will listen on the ReferenceQueue and remove the entry in the @@ -85,21 +93,23 @@ public class SoftRefFilesCache extends A continue; } - FileSystemAndNameKey key; - synchronized (refReverseMap) + lock.lock(); + try { - key = refReverseMap.get(ref); - } + FileSystemAndNameKey key = refReverseMap.get(ref); - if (key != null) - { - if (removeFile(key)) + if (key != null) { - /* This is not thread safe - filesystemClose(key.getFileSystem()); - */ + if (removeFile(key)) + { + filesystemClose(key.getFileSystem()); + } } } + finally + { + lock.unlock(); + } } catch (InterruptedException e) { @@ -120,23 +130,31 @@ public class SoftRefFilesCache extends A private void startThread() { - if (softRefReleaseThread != null) + Thread thread; + SoftRefReleaseThread newThread; + do + { + newThread = null; + thread = softRefReleaseThread.get(); + if (thread != null) + { + break; + } + newThread = new SoftRefReleaseThread(); + } while (softRefReleaseThread.compareAndSet(null, newThread)); + if (newThread != null) { - throw new IllegalStateException( - Messages.getString("vfs.impl/SoftRefReleaseThread-already-running.warn")); + newThread.start(); } - - softRefReleaseThread = new SoftRefReleaseThread(); - softRefReleaseThread.start(); } private void endThread() { - if (softRefReleaseThread != null) + SoftRefReleaseThread thread = softRefReleaseThread.getAndSet(null); + if (thread != null) { - softRefReleaseThread.requestEnd = true; - softRefReleaseThread.interrupt(); - softRefReleaseThread = null; + thread.requestEnd = true; + thread.interrupt(); } } @@ -150,20 +168,55 @@ public class SoftRefFilesCache extends A Map<FileName, Reference<FileObject>> files = getOrCreateFilesystemCache(file.getFileSystem()); Reference<FileObject> ref = createReference(file, refqueue); - FileSystemAndNameKey key = new FileSystemAndNameKey(file - .getFileSystem(), file.getName()); + FileSystemAndNameKey key = new FileSystemAndNameKey(file.getFileSystem(), file.getName()); - synchronized (files) + lock.lock(); + try { Reference<FileObject> old = files.put(file.getName(), ref); - synchronized (refReverseMap) + if (old != null) { - if (old != null) - { - refReverseMap.remove(old); - } - refReverseMap.put(ref, key); + refReverseMap.remove(old); + } + refReverseMap.put(ref, key); + } + finally + { + lock.unlock(); + } + } + + + public boolean putFileIfAbsent(final FileObject file) + { + if (log.isDebugEnabled()) + { + log.debug("putFile: " + file.getName()); + } + + Map<FileName, Reference<FileObject>> files = getOrCreateFilesystemCache(file.getFileSystem()); + + Reference<FileObject> ref = createReference(file, refqueue); + FileSystemAndNameKey key = new FileSystemAndNameKey(file.getFileSystem(), file.getName()); + + lock.lock(); + try + { + if (files.containsKey(file.getName()) && files.get(file.getName()).get() != null) + { + return false; + } + Reference<FileObject> old = files.put(file.getName(), ref); + if (old != null) + { + refReverseMap.remove(old); } + refReverseMap.put(ref, key); + return true; + } + finally + { + lock.unlock(); } } @@ -176,7 +229,8 @@ public class SoftRefFilesCache extends A { Map<FileName, Reference<FileObject>> files = getOrCreateFilesystemCache(filesystem); - synchronized (files) + lock.lock(); + try { Reference<FileObject> ref = files.get(name); if (ref == null) @@ -191,55 +245,61 @@ public class SoftRefFilesCache extends A } return fo; } + finally + { + lock.unlock(); + } } public void clear(FileSystem filesystem) { Map<FileName, Reference<FileObject>> files = getOrCreateFilesystemCache(filesystem); - boolean closeFilesystem; - - synchronized (files) + lock.lock(); + try { - synchronized (refReverseMap) + Iterator<FileSystemAndNameKey> iterKeys = refReverseMap.values().iterator(); + while (iterKeys.hasNext()) { - Iterator<FileSystemAndNameKey> iterKeys = refReverseMap.values().iterator(); - while (iterKeys.hasNext()) + FileSystemAndNameKey key = iterKeys.next(); + if (key.getFileSystem() == filesystem) { - FileSystemAndNameKey key = iterKeys.next(); - if (key.getFileSystem() == filesystem) - { - iterKeys.remove(); - files.remove(key.getFileName()); - } + iterKeys.remove(); + files.remove(key.getFileName()); } + } - closeFilesystem = files.size() < 1; + if (filesystemCache.size() < 1) + { + filesystemClose(filesystem); } } - - if (closeFilesystem) + finally { - filesystemClose(filesystem); + lock.unlock(); } } + /** + * Called while the lock is held + * @param filesystem The file system to close. + */ private void filesystemClose(FileSystem filesystem) { if (log.isDebugEnabled()) { log.debug("close fs: " + filesystem.getRootName()); } - synchronized (filesystemCache) + + filesystemCache.remove(filesystem); + if (filesystemCache.size() < 1) { - filesystemCache.remove(filesystem); - if (filesystemCache.size() < 1) - { - endThread(); - } + endThread(); } + /* This is not thread-safe as another thread might be opening the file system ((DefaultFileSystemManager) getContext().getFileSystemManager()) ._closeFileSystem(filesystem); + */ } @Override @@ -249,16 +309,17 @@ public class SoftRefFilesCache extends A endThread(); - // files.clear(); - synchronized (filesystemCache) + lock.lock(); + try { filesystemCache.clear(); - } - synchronized (refReverseMap) - { refReverseMap.clear(); } + finally + { + lock.unlock(); + } } public void removeFile(FileSystem filesystem, FileName name) @@ -282,38 +343,42 @@ public class SoftRefFilesCache extends A Map<?, ?> files = getOrCreateFilesystemCache(key.getFileSystem()); - synchronized (files) + lock.lock(); + try { Object ref = files.remove(key.getFileName()); if (ref != null) { - synchronized (refReverseMap) - { - refReverseMap.remove(ref); - } + refReverseMap.remove(ref); } return files.size() < 1; } + finally + { + lock.unlock(); + } } protected Map<FileName, Reference<FileObject>> getOrCreateFilesystemCache(final FileSystem filesystem) { - synchronized (filesystemCache) + if (filesystemCache.size() < 1) { - if (filesystemCache.size() < 1) - { - startThread(); - } + startThread(); + } + + Map<FileName, Reference<FileObject>> files; - Map<FileName, Reference<FileObject>> files = filesystemCache.get(filesystem); - if (files == null) + do + { + files = filesystemCache.get(filesystem); + if (files != null) { - files = new HashMap<FileName, Reference<FileObject>>(); - filesystemCache.put(filesystem, files); + break; } + files = new HashMap<FileName, Reference<FileObject>>(); + } while (filesystemCache.putIfAbsent(filesystem, files) == null); - return files; - } + return files; } }