From 88e0399f3a39fe1c22053ac3c0cfc03ab826754f Mon Sep 17 00:00:00 2001
From: Nathaniel Wesley Filardo <nwf@cs.jhu.edu>
Date: Sat, 17 Sep 2016 15:46:56 -0400
Subject: [PATCH] AudioProvider hot path a little smaller

Mutation to the collection of receivers now happens on cloned objects
and so the only synchronized operation needed on the hot path is the
read of the current pointer, rather than a full clone.  Immutability changes
everything.
---
 .../watchviz/vizlib/AudioProvider.java         | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/vizlib/src/main/java/com/acmetensortoys/watchviz/vizlib/AudioProvider.java b/vizlib/src/main/java/com/acmetensortoys/watchviz/vizlib/AudioProvider.java
index cbd7e98..8e5f664 100644
--- a/vizlib/src/main/java/com/acmetensortoys/watchviz/vizlib/AudioProvider.java
+++ b/vizlib/src/main/java/com/acmetensortoys/watchviz/vizlib/AudioProvider.java
@@ -23,7 +23,8 @@ public class AudioProvider {
     public void add(AudioReceiver ar) {
         synchronized(this) {
             boolean empty = audioReceivers.isEmpty();
-            boolean did = audioReceivers.add(ar);
+            Set<AudioReceiver> nextar = new HashSet<>(audioReceivers);
+            boolean did = nextar.add(ar);
 
             if (empty) {
                 Log.d("AudioProvider", "Starting audio source thread...");
@@ -31,16 +32,19 @@ public class AudioProvider {
             } else if (did) {
                 Log.d("AudioProvider", "Added receiver; now=" + audioReceivers.size());
             }
+            audioReceivers = nextar;
         }
     }
     public void remove(AudioReceiver ar) {
         synchronized(this) {
-            boolean did = audioReceivers.remove(ar);
+            Set<AudioReceiver> nextar = new HashSet<>(audioReceivers);
+            boolean did = nextar.remove(ar);
             if(!did) {
-                // Nothing to do!
+                // Nothing to do, no need to update anything.
                 return;
             }
-            if(audioReceivers.isEmpty()) {
+            audioReceivers = nextar;
+            if(nextar.isEmpty()) {
                 Log.d("AudioProvider", "Stopping audio source thread...");
                 this.stop();
             } else {
@@ -70,16 +74,14 @@ public class AudioProvider {
 
                 ar.startRecording();
                 while (!Thread.interrupted()) {
-                    final Rendering rcb;
                     ar.read(samples, 0, samples.length, AudioRecord.READ_BLOCKING);
                     System.arraycopy(samples, 0, fft, 0, AUDIO_SAMPLES);
                     fftc.realForward(fft);
 
-                    // Snapshot the current callbacks and iterate that.  This prevents
-                    // concurrent mutation exceptions, at the cost of being kind of terrible.
+                    // Grab a pointer to the current set and iterate that.
                     Set<AudioReceiver> ars;
                     synchronized(this) {
-                        ars = new HashSet<>(audioReceivers);
+                        ars = audioReceivers;
                     }
                     for (AudioReceiver ar : ars) {
                         ar.onAudio(samples, fft);
-- 
2.50.1