Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better lazy resolution #91

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</parent>

<artifactId>scijava-common</artifactId>
<version>2.41.1-SNAPSHOT</version>
<version>3.0.0-SNAPSHOT</version>

<name>SciJava Common</name>
<description>SciJava Common is a shared library for SciJava software. It provides a plugin framework, with an extensible mechanism for service discovery, backed by its own annotation processor, so that plugins can be loaded dynamically. It is used by both ImageJ and SCIFIO.</description>
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/scijava/object/LazyObjects.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,11 @@ public interface LazyObjects<T> {
/** Gets the collection of objects. */
Collection<T> get();

/**
* The type of the objects which will be resolved from a call to
* {@link #get()}. This information is used to determine whether to resolve
* the objects in response to an {@link ObjectIndex#get(Class)} call.
*/
Class<?> getType();

}
25 changes: 21 additions & 4 deletions src/main/java/org/scijava/object/ObjectIndex.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public List<E> getAll() {
*/
public List<E> get(final Class<?> type) {
// lazily register any pending objects
if (!pending.isEmpty()) resolvePending();
if (!pending.isEmpty()) resolvePending(type);

List<E> list = retrieveList(type);
// NB: Return a copy of the data, to facilitate thread safety.
Expand Down Expand Up @@ -380,15 +380,32 @@ protected List<E> retrieveList(final Class<?> type) {
return list;
}

private void resolvePending() {
private void resolvePending(final Class<?> type) {
synchronized (pending) {
while (!pending.isEmpty()) {
final LazyObjects<? extends E> c = pending.remove(0);
for (int p = pending.size() - 1; p >= 0; p--) {
final LazyObjects<? extends E> c = pending.get(p);

// NB: If this pending callback returns objects of a different
// type than the one we are interested in, it can be skipped.
if (!isCompatibleType(c, type)) continue;

// trigger the callback and add the results
pending.remove(p);
addAll(c.get());
}
}
}

// -- Helper methods --

private boolean isCompatibleType(final LazyObjects<? extends E> c,
final Class<?> type)
{
if (type == All.class) return true;
final Class<?> cType = c.getType();
return cType.isAssignableFrom(type) || type.isAssignableFrom(cType);
}

// -- Helper classes --

private static class All {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public boolean registerAppMenus(final Object menus) {
}

@Override
public List<? extends Platform> filterInstances(final List<Platform> list) {
public List<Platform> filterInstances(final List<Platform> list) {
final Iterator<Platform> iter = list.iterator();
while (iter.hasNext()) {
if (!iter.next().isTarget()) {
Expand Down
45 changes: 25 additions & 20 deletions src/main/java/org/scijava/plugin/AbstractSingletonService.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@

package org.scijava.plugin;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -58,19 +57,16 @@ public abstract class AbstractSingletonService<PT extends SingletonPlugin>
private ObjectService objectService;

// TODO: Listen for PluginsAddedEvent and PluginsRemovedEvent
// and update the list of singletons accordingly.

/** List of singleton plugin instances. */
private List<PT> instances;
// and update the map of singletons accordingly.

private Map<Class<? extends PT>, PT> instanceMap;

// -- SingletonService methods --

@Override
public List<PT> getInstances() {
if (instances == null) initInstances();
return instances;
final List<PT> plugins = objectService.getObjects(getPluginType());
return Collections.unmodifiableList(plugins);
}

@SuppressWarnings("unchecked")
Expand All @@ -85,12 +81,18 @@ public <P extends PT> P getInstance(final Class<P> pluginClass) {
@Override
public void initialize() {
// add singleton instances to the object index... IN THE FUTURE!
objectService.getIndex().addLater(new LazyObjects<Object>() {
objectService.getIndex().addLater(new LazyObjects<PT>() {

@Override
public ArrayList<Object> get() {
return new ArrayList<Object>(getInstances());
public List<PT> get() {
return createInstances();
}

@Override
public Class<?> getType() {
return getPluginType();
}

});
}

Expand All @@ -102,34 +104,37 @@ public ArrayList<Object> get() {
* @param list the initial list of instances
* @return the filtered list of instances
*/
protected List<? extends PT> filterInstances(final List<PT> list) {
protected List<PT> filterInstances(final List<PT> list) {
return list;
}

// -- Helper methods --

private synchronized void initInstances() {
if (instances != null) return;

final List<PT> list =
Collections.unmodifiableList(filterInstances(getPluginService()
.createInstancesOfType(getPluginType())));
if (instanceMap != null) return;

final HashMap<Class<? extends PT>, PT> map =
new HashMap<Class<? extends PT>, PT>();

final List<PT> list = getInstances();
for (final PT plugin : list) {
@SuppressWarnings("unchecked")
final Class<? extends PT> ptClass =
(Class<? extends PT>) plugin.getClass();
map.put(ptClass, plugin);
}

log.debug("Found " + list.size() + " " + getPluginType().getSimpleName() +
" plugins.");

instanceMap = map;
instances = list;
}

private List<PT> createInstances() {
final List<PT> instances =
filterInstances(getPluginService().createInstancesOfType(getPluginType()));

log.info("Found " + instances.size() + " " +
getPluginType().getSimpleName() + " plugins.");

return instances;
}

}
5 changes: 5 additions & 0 deletions src/main/java/org/scijava/script/DefaultScriptService.java
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@ public Collection<ScriptInfo> get() {
return scripts().values();
}

@Override
public Class<?> getType() {
return ScriptInfo.class;
}

});
}

Expand Down
118 changes: 117 additions & 1 deletion src/test/java/org/scijava/object/ObjectIndexTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;

Expand All @@ -60,11 +63,30 @@ public void testGetAll() {
objectIndex.add(o1);
objectIndex.add(o2);
objectIndex.add(o3);

final String o4 = "quick", o5 = "brown", o6 = "fox";
objectIndex.addLater(new LazyObjects<String>() {

@Override
public Collection<String> get() {
return Arrays.asList(o4, o5, o6);
}

@Override
public Class<?> getType() {
return String.class;
}

});

final List<Object> all = objectIndex.getAll();
assertEquals(3, all.size());
assertEquals(6, all.size());
assertSame(o1, all.get(0));
assertSame(o2, all.get(1));
assertSame(o3, all.get(2));
assertSame(o4, all.get(3));
assertSame(o5, all.get(4));
assertSame(o6, all.get(5));
}

@Test
Expand Down Expand Up @@ -222,4 +244,98 @@ public void testToString() {
assertArrayEquals(expected, actual);
}

@Test
public void testAddLater() {
final ObjectIndex<Object> objectIndex =
new ObjectIndex<Object>(Object.class);
objectIndex.add(new Integer(5));
objectIndex.add(new Float(2.5f));
objectIndex.add(new Integer(3));

final LazyThings<Integer> lazyIntegers = new LazyThings<Integer>(9, -7);
objectIndex.addLater(lazyIntegers);

final LazyThings<Float> lazyFloats =
new LazyThings<Float>(6.6f, -3.3f, -5.1f, 12.3f);
objectIndex.addLater(lazyFloats);

final LazyThings<BigInteger> lazyBigIntegers =
new LazyThings<BigInteger>(BigInteger.ONE, BigInteger.TEN);
objectIndex.addLater(lazyBigIntegers);

// verify that no pending objects have been resolved yet
assertFalse(lazyIntegers.wasAccessed());
assertFalse(lazyFloats.wasAccessed());
assertFalse(lazyBigIntegers.wasAccessed());

// verify list of Integers; this will resolve the pending ones
final List<Object> integerObjects = objectIndex.get(Integer.class);
assertEquals(4, integerObjects.size());
assertEquals(5, integerObjects.get(0));
assertEquals(3, integerObjects.get(1));
assertEquals(9, integerObjects.get(2));
assertEquals(-7, integerObjects.get(3));

// verify that pending Integers have now been resolved
assertTrue(lazyIntegers.wasAccessed());

// verify that the other pending objects have still not been resolved
assertFalse(lazyFloats.wasAccessed());
assertFalse(lazyBigIntegers.wasAccessed());

// verify list of Floats; this will resolve the pending ones
final List<Object> floatObjects = objectIndex.get(Float.class);
assertEquals(5, floatObjects.size());
assertEquals(2.5f, floatObjects.get(0));
assertEquals(6.6f, floatObjects.get(1));
assertEquals(-3.3f, floatObjects.get(2));
assertEquals(-5.1f, floatObjects.get(3));
assertEquals(12.3f, floatObjects.get(4));

// verify that pending Floats have now been resolved
assertTrue(lazyFloats.wasAccessed());

// verify that pending BigIntegers have still not been resolved
assertFalse(lazyBigIntegers.wasAccessed());

// verify list of BigIntegers; this will resolve the pending ones
final List<Object> bigIntegerObjects = objectIndex.get(BigInteger.class);
assertEquals(2, bigIntegerObjects.size());
assertEquals(BigInteger.ONE, bigIntegerObjects.get(0));
assertEquals(BigInteger.TEN, bigIntegerObjects.get(1));

// verify that pending BigIntegers have finally been resolved
assertTrue(lazyBigIntegers.wasAccessed());
}

// -- Helper classes --

public static class LazyThings<T> implements LazyObjects<T> {

private Collection<T> objects;
private Class<?> type;
private boolean accessed;

public LazyThings(T... objects) {
this.objects = Arrays.asList(objects);
this.type = objects[0].getClass();
}

@Override
public Collection<T> get() {
accessed = true;
return objects;
}

@Override
public Class<?> getType() {
return type;
}

public boolean wasAccessed() {
return accessed;
}

}

}