Improve concurrency by narrowing the RankedBindings subscription lock down to the publisher.
Also fix corner case where a publisher that has a maximum binding rank of Integer.MIN_VALUE
might never be called, despite that being a valid rank.
diff --git a/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedBindings.java b/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedBindings.java
index ecd06fc..23aee30 100644
--- a/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedBindings.java
+++ b/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedBindings.java
@@ -35,8 +35,6 @@
final Collection<BeanCache<?, T>> cachedBeans = Weak.elements();
- volatile int topRank = Integer.MAX_VALUE;
-
// ----------------------------------------------------------------------
// Constructors
// ----------------------------------------------------------------------
@@ -105,10 +103,6 @@
* No need to lock; ranked sequence is thread-safe.
*/
pendingPublishers.insert( publisher, rank );
- if ( rank > topRank )
- {
- topRank = rank;
- }
}
void remove( final BindingPublisher publisher )
@@ -116,7 +110,7 @@
/*
* Lock just to prevent subscription race condition.
*/
- synchronized ( pendingPublishers )
+ synchronized ( publisher )
{
if ( !pendingPublishers.removeThis( publisher ) )
{
@@ -147,17 +141,21 @@
public boolean hasNext()
{
- int rank = topRank;
- if ( rank > Integer.MIN_VALUE && rank > itr.peekNextRank() )
+ // apply any publishers that could add bindings before the current position
+ BindingPublisher publisher = pendingPublishers.peek();
+ while ( null != publisher && !itr.hasNext( publisher.maxBindingRank() ) )
{
- synchronized ( pendingPublishers )
+ synchronized ( publisher )
{
- while ( ( rank = pendingPublishers.topRank() ) > Integer.MIN_VALUE && rank > itr.peekNextRank() )
+ // check in case subscribed by another thread
+ if ( publisher == pendingPublishers.peek() )
{
- pendingPublishers.poll().subscribe( RankedBindings.this );
+ // only update list _after_ subscription
+ publisher.subscribe( RankedBindings.this );
+ pendingPublishers.removeThis( publisher );
}
- topRank = rank;
}
+ publisher = pendingPublishers.peek();
}
return itr.hasNext();
}
diff --git a/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedSequence.java b/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedSequence.java
index 1e346c4..451e65d 100644
--- a/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedSequence.java
+++ b/org.eclipse.sisu.inject/src/org/eclipse/sisu/inject/RankedSequence.java
@@ -70,17 +70,10 @@
}
@SuppressWarnings( "unchecked" )
- public T poll()
+ public T peek()
{
final Content content = get();
- set( content.remove( 0 ) );
- return (T) content.objs[0];
- }
-
- public int topRank()
- {
- final Content content = get();
- return null != content ? uid2rank( content.uids[0] ) : Integer.MIN_VALUE;
+ return null != content ? (T) content.objs[0] : null;
}
public boolean contains( final Object element )
@@ -391,13 +384,13 @@
}
/**
- * @return Rank assigned to the next element; returns {@link Integer#MIN_VALUE} if there is no next element.
+ * @return {@code true} if the next element is ranked at or above the given rank; otherwise {@code false}
*/
- public int peekNextRank()
+ public boolean hasNext( final int rank )
{
if ( null != nextObj )
{
- return uid2rank( nextUID );
+ return uid2rank( nextUID ) >= rank;
}
final Content newContent = get();
if ( content != newContent )
@@ -407,9 +400,9 @@
}
if ( index >= 0 && index < content.uids.length )
{
- return uid2rank( content.uids[index] );
+ return uid2rank( content.uids[index] ) >= rank;
}
- return Integer.MIN_VALUE;
+ return false;
}
public T next()
