Avoid concurrency issues

hinerm · ctrueden · commit 033a921a8516 · 2015-03-20T22:57:13.000-05:00
When streams are added or removed, we do not want that to interfere with
concurrent stream operations (i.e., writing, closing and flushing).
But we also do not particularly want to incur the heavy performance
penalty of synchronizing all the stream operations, either. So let's
keep a copy of the streams, which is regenerated whenever stream is
added or removed from the mix, and use that copy when iterating over the
streams during the stream operations.

Similarly, when listeners are added or removed from the ConsoleService,
we use the same trick to avoid the need to synchronize listener
notification every time an output event occurs.

Signed-off-by: Curtis Rueden &lt;ctrueden@wisc.edu&gt;
diff --git a/src/main/java/org/scijava/console/DefaultConsoleService.java b/src/main/java/org/scijava/console/DefaultConsoleService.java
@@ -69,6 +69,8 @@ public class DefaultConsoleService extends
 	/** List of listeners for {@code stdout} and {@code stderr} output. */
 	private ArrayList<OutputListener> listeners;
 
+	private OutputListener[] cachedListeners;
+
 	// -- ConsoleService methods --
 
 	@Override
@@ -97,6 +99,7 @@ public void addOutputListener(final OutputListener l) {
 		if (listeners == null) initListeners();
 		synchronized (listeners) {
 			listeners.add(l);
+			cacheListeners();
 		}
 	}
 
@@ -105,17 +108,16 @@ public void removeOutputListener(final OutputListener l) {
 		if (listeners == null) initListeners();
 		synchronized (listeners) {
 			listeners.remove(l);
+			cacheListeners();
 		}
 	}
 
 	@Override
 	public void notifyListeners(final OutputEvent event) {
 		if (listeners == null) initListeners();
-		synchronized (listeners) {
-			for (final OutputListener l : listeners) {
-				l.outputOccurred(event);
-			}
-		}
+		final OutputListener[] toNotify = cachedListeners;
+		for (final OutputListener l : toNotify)
+			l.outputOccurred(event);
 	}
 
 	// -- PTService methods --
@@ -158,10 +160,15 @@ private synchronized void initListeners() {
 		syserr.getParent().addOutputStream(err);
 
 		listeners = new ArrayList<OutputListener>();
+		cachedListeners = listeners.toArray(new OutputListener[0]);
 	}
 
 	// -- Helper methods --
 
+	private void cacheListeners() {
+		cachedListeners = listeners.toArray(new OutputListener[listeners.size()]);
+	}
+
 	private MultiPrintStream multiPrintStream(final PrintStream ps) {
 		if (ps instanceof MultiPrintStream) return (MultiPrintStream) ps;
 		return new MultiPrintStream(ps);
diff --git a/src/main/java/org/scijava/console/MultiOutputStream.java b/src/main/java/org/scijava/console/MultiOutputStream.java
@@ -50,6 +50,8 @@ public class MultiOutputStream extends OutputStream {
 
 	private final ArrayList<OutputStream> streams;
 
+	private OutputStream[] cachedStreams;
+
 	/**
 	 * Forwards output to a list of output streams.
 	 *
@@ -60,6 +62,7 @@ public MultiOutputStream(final OutputStream... os) {
 		for (int i = 0; i < os.length; i++) {
 			streams.add(os[i]);
 		}
+		cacheStreams();
 	}
 
 	// -- MultiOutputStream methods --
@@ -68,46 +71,58 @@ public MultiOutputStream(final OutputStream... os) {
 	public void addOutputStream(final OutputStream os) {
 		synchronized (streams) {
 			streams.add(os);
+			cacheStreams();
 		}
 	}
 
 	/** Removes an output stream from those receiving this stream's output. */
 	public void removeOutputStream(final OutputStream os) {
 		synchronized (streams) {
 			streams.remove(os);
+			cacheStreams();
 		}
 	}
 
 	// -- OutputStream methods --
 
 	@Override
 	public void write(final int b) throws IOException {
-		for (final OutputStream stream : streams)
+		final OutputStream[] toWrite = cachedStreams;
+		for (final OutputStream stream : toWrite)
 			stream.write(b);
 	}
 
 	@Override
 	public void write(final byte[] buf, final int off, final int len)
 		throws IOException
 	{
-		for (final OutputStream stream : streams)
+		final OutputStream[] toWrite = cachedStreams;
+		for (final OutputStream stream : toWrite)
 			stream.write(buf, off, len);
 	}
 
 	// -- Closeable methods --
 
 	@Override
 	public void close() throws IOException {
-		for (final OutputStream stream : streams)
+		final OutputStream[] toClose = cachedStreams;
+		for (final OutputStream stream : toClose)
 			stream.close();
 	}
 
 	// -- Flushable methods --
 
 	@Override
 	public void flush() throws IOException {
-		for (final OutputStream stream : streams)
+		final OutputStream[] toFlush = cachedStreams;
+		for (final OutputStream stream : toFlush)
 			stream.flush();
 	}
 
+	// -- Helper methods --
+
+	private void cacheStreams() {
+		cachedStreams = streams.toArray(new OutputStream[streams.size()]);
+	}
+
 }

Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,8 @@ public class MultiOutputStream extends OutputStream {`
`50`	`50`
`51`	`51`	`private final ArrayList<OutputStream> streams;`
`52`	`52`
	`53`	`+ private OutputStream[] cachedStreams;`
	`54`	`+`
`53`	`55`	`/**`
`54`	`56`	`* Forwards output to a list of output streams.`
`55`	`57`	`*`
`@@ -60,6 +62,7 @@ public MultiOutputStream(final OutputStream... os) {`
`60`	`62`	`for (int i = 0; i < os.length; i++) {`
`61`	`63`	`streams.add(os[i]);`
`62`	`64`	`}`
	`65`	`+ cacheStreams();`
`63`	`66`	`}`
`64`	`67`
`65`	`68`	`// -- MultiOutputStream methods --`
`@@ -68,46 +71,58 @@ public MultiOutputStream(final OutputStream... os) {`
`68`	`71`	`public void addOutputStream(final OutputStream os) {`
`69`	`72`	`synchronized (streams) {`
`70`	`73`	`streams.add(os);`
	`74`	`+ cacheStreams();`
`71`	`75`	`}`
`72`	`76`	`}`
`73`	`77`
`74`	`78`	`/** Removes an output stream from those receiving this stream's output. */`
`75`	`79`	`public void removeOutputStream(final OutputStream os) {`
`76`	`80`	`synchronized (streams) {`
`77`	`81`	`streams.remove(os);`
	`82`	`+ cacheStreams();`
`78`	`83`	`}`
`79`	`84`	`}`
`80`	`85`
`81`	`86`	`// -- OutputStream methods --`
`82`	`87`
`83`	`88`	`@Override`
`84`	`89`	`public void write(final int b) throws IOException {`
`85`		`- for (final OutputStream stream : streams)`
	`90`	`+ final OutputStream[] toWrite = cachedStreams;`
	`91`	`+ for (final OutputStream stream : toWrite)`
`86`	`92`	`stream.write(b);`
`87`	`93`	`}`
`88`	`94`
`89`	`95`	`@Override`
`90`	`96`	`public void write(final byte[] buf, final int off, final int len)`
`91`	`97`	`throws IOException`
`92`	`98`	`{`
`93`		`- for (final OutputStream stream : streams)`
	`99`	`+ final OutputStream[] toWrite = cachedStreams;`
	`100`	`+ for (final OutputStream stream : toWrite)`
`94`	`101`	`stream.write(buf, off, len);`
`95`	`102`	`}`
`96`	`103`
`97`	`104`	`// -- Closeable methods --`
`98`	`105`
`99`	`106`	`@Override`
`100`	`107`	`public void close() throws IOException {`
`101`		`- for (final OutputStream stream : streams)`
	`108`	`+ final OutputStream[] toClose = cachedStreams;`
	`109`	`+ for (final OutputStream stream : toClose)`
`102`	`110`	`stream.close();`
`103`	`111`	`}`
`104`	`112`
`105`	`113`	`// -- Flushable methods --`
`106`	`114`
`107`	`115`	`@Override`
`108`	`116`	`public void flush() throws IOException {`
`109`		`- for (final OutputStream stream : streams)`
	`117`	`+ final OutputStream[] toFlush = cachedStreams;`
	`118`	`+ for (final OutputStream stream : toFlush)`
`110`	`119`	`stream.flush();`
`111`	`120`	`}`
`112`	`121`
	`122`	`+ // -- Helper methods --`
	`123`	`+`
	`124`	`+ private void cacheStreams() {`
	`125`	`+ cachedStreams = streams.toArray(new OutputStream[streams.size()]);`
	`126`	`+ }`
	`127`	`+`
`113`	`128`	`}`