Skip to content

Commit

Permalink
add local implementation of SoftRefLoaderCache that addresses a cache…
Browse files Browse the repository at this point in the history
… synchronization issue (imglib/imglib2-cache#22) -- which we will fix later differently
  • Loading branch information
StephanPreibisch committed Mar 28, 2023
1 parent cdd0bb5 commit 36f5698
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/main/java/org/janelia/saalfeldlab/hotknife/util/Lazy.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import net.imglib2.cache.img.CachedCellImg;
import net.imglib2.cache.img.CellLoader;
import net.imglib2.cache.img.LoadedCellCacheLoader;
import net.imglib2.cache.ref.SoftRefLoaderCache;
import net.imglib2.img.basictypeaccess.AccessFlags;
import net.imglib2.img.basictypeaccess.ArrayDataAccessFactory;
import net.imglib2.img.cell.Cell;
Expand All @@ -55,6 +54,8 @@

/**
* Convenience methods to create lazy evaluated cached cell images with ops or consumers.
*
* This is a re-implementation that fixes concurrency issues when calling invalidateAll() while BDV is drawing
*
* @author Stephan Saalfeld <[email protected]>
*/
Expand Down Expand Up @@ -120,7 +121,7 @@ private Lazy() {}

@SuppressWarnings({"unchecked", "rawtypes"})
final Cache<Long, Cell<?>> cache =
new SoftRefLoaderCache().withLoader(LoadedCellCacheLoader.get(grid, loader, type, accessFlags));
new RobustSoftRefLoaderCache().withLoader(LoadedCellCacheLoader.get(grid, loader, type, accessFlags));

return createImg(grid, cache, type, accessFlags);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
package org.janelia.saalfeldlab.hotknife.util;

import java.lang.ref.ReferenceQueue;
import java.lang.ref.SoftReference;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.function.Predicate;

import net.imglib2.cache.CacheLoader;
import net.imglib2.cache.LoaderCache;

public class RobustSoftRefLoaderCache<K, V> implements LoaderCache< K, V >
{
final ConcurrentHashMap< K, Entry > map = new ConcurrentHashMap<>();

final ReferenceQueue< V > queue = new ReferenceQueue<>();

static final class CacheSoftReference< V > extends SoftReference< V >
{
private final RobustSoftRefLoaderCache< ?, V >.Entry entry;

public CacheSoftReference()
{
super( null );
this.entry = null;
}

public CacheSoftReference( final V referent, final ReferenceQueue< V > remove, final RobustSoftRefLoaderCache< ?, V >.Entry entry )
{
super( referent, remove );
this.entry = entry;
}
}

final class Entry
{
final K key;

private CacheSoftReference< V > ref;

boolean loaded;

public Entry( final K key )
{
this.key = key;
this.ref = new CacheSoftReference<>();
this.loaded = false;
}

public V getValue()
{
// instead of synchronize statement
final CacheSoftReference< V > myRef = ref;
if ( myRef == null )
return null;
else
return myRef.get();

// old code (Problem: myRef can be null when calling invalidateAll()
// return myRef.get();
}

public void setValue( final V value )
{
this.loaded = true;
this.ref = new CacheSoftReference<>( value, queue, this );
}

public void remove()
{
map.remove( key, this );
}
}

@Override
public V getIfPresent( final K key )
{
cleanUp();
final Entry entry = map.get( key );
return entry == null ? null : entry.getValue();
}

@Override
public V get( final K key, final CacheLoader< ? super K, ? extends V > loader ) throws ExecutionException
{
cleanUp();
final Entry entry = map.computeIfAbsent( key, ( k ) -> new Entry( k ) );
V value = entry.getValue();
if ( value == null )
{
synchronized ( entry )
{
if ( entry.loaded )
{
value = entry.getValue();
if ( value == null )
{
/*
* The entry was already loaded, but its value has been
* garbage collected. We need to create a new entry
*/
entry.remove();
value = get( key, loader );
}
}
else
{
try
{
value = loader.get( key );
entry.setValue( value );
}
catch ( final InterruptedException e )
{
Thread.currentThread().interrupt();
throw new ExecutionException( e );
}
catch ( final Exception e )
{
throw new ExecutionException( e );
}
}
}
}
return value;
}

@Override
public void persist( final K key )
{}

@Override
public void persistIf( final Predicate< K > condition )
{}

@Override
public void persistAll()
{}

@Override
public void invalidate( final K key )
{
final Entry entry = map.remove( key );
if ( entry != null )
{
final CacheSoftReference< V > ref = entry.ref;
if ( ref != null )
ref.clear();
entry.ref = null;

This comment has been minimized.

Copy link
@tpietzsch

tpietzsch Mar 29, 2023

Collaborator

Would be better to not set entry.ref = null here, instead of checking everytime in getValue() above

}
}

@Override
public void invalidateIf( final long parallelismThreshold, final Predicate< K > condition )
{
map.forEachValue( parallelismThreshold, entry ->
{
if ( condition.test( entry.key ) )
{
entry.remove();
final CacheSoftReference< V > ref = entry.ref;
if ( ref != null )
ref.clear();
entry.ref = null;

This comment has been minimized.

Copy link
@tpietzsch

tpietzsch Mar 29, 2023

Collaborator

and here

}
} );
}

@Override
public void invalidateAll( final long parallelismThreshold )
{
// TODO: We could also simply do map.clear(). Pros/Cons?

map.forEachValue( parallelismThreshold, entry ->
{
entry.remove();
final CacheSoftReference< V > ref = entry.ref;
if ( ref != null )
ref.clear();
entry.ref = null;

This comment has been minimized.

Copy link
@tpietzsch

tpietzsch Mar 29, 2023

Collaborator

and here

} );
}

/**
* Remove entries from the cache whose references have been
* garbage-collected.
*/
public void cleanUp()
{
while ( true )
{
@SuppressWarnings( "unchecked" )
final CacheSoftReference< V > poll = ( CacheSoftReference< V > ) queue.poll();
if ( poll == null )
break;
poll.entry.remove();
}
}

}

0 comments on commit 36f5698

Please sign in to comment.