Implemented unsafe asynchronous mode, allowing the bot to read directly from live game data while the frame buffer is being populated. There's some additional cruft lying around and there's no guarantee that this will actually work in practice, but the test case is passing.

dgant · dgant · commit 8bb4f8054c7c · 2020-09-21T16:11:27.000-04:00
diff --git a/src/main/java/bwapi/BWClient.java b/src/main/java/bwapi/BWClient.java
@@ -93,12 +93,25 @@ public void startGame(BWClientConfiguration configuration) {
                 }
             }
             while (liveGameData.isInGame()) {
+                boolean timeFrame = liveGameData.getFrameCount() > 0 || ! configuration.unlimitedFrameZero;
+                performanceMetrics.frameDuration5.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration10.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration15.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration20.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration25.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration30.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration35.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration40.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration45.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration50.timeIf(timeFrame, () ->
+                performanceMetrics.frameDuration55.timeIf(timeFrame, () ->
                 performanceMetrics.totalFrameDuration.timeIf(
-                    liveGameData.getFrameCount() > 0 || ! configuration.unlimitedFrameZero,
+                    timeFrame,
                     () -> {
                         botWrapper.onFrame();
                         performanceMetrics.flushSideEffects.time(() -> getGame().sideEffects.flushTo(liveGameData));
-                    });
+                    })
+                )))))))))));
                 performanceMetrics.bwapiResponse.time(client::update);
                 if (!client.isConnected()) {
                     System.out.println("Reconnecting...");
diff --git a/src/main/java/bwapi/BWClientConfiguration.java b/src/main/java/bwapi/BWClientConfiguration.java
@@ -50,7 +50,16 @@ public class BWClientConfiguration {
      * The maximum number of frames to buffer while waiting on a bot.
      * Each frame buffered adds about 33 megabytes to JBWAPI's memory footprint.
      */
-    public int asyncFrameBufferSize = 10;
+    public int asyncFrameBufferCapacity = 10;
+
+    /**
+     * Enables thread-unsafe async mode.
+     * In this mode, the bot is allowed to read directly from shared memory until shared memory has been copied into the frame buffer,
+     * at wihch point the bot switches to using the frame buffer.
+     * This should enhance performance by allowing the bot to act while the frame is copied, but poses unidentified risk due to
+     * the non-thread-safe switc from shared memory reads to frame buffer reads.
+     */
+    public boolean asyncUnsafe = false;
 
     /**
      * Toggles verbose logging, particularly of synchronization steps.
@@ -64,8 +73,8 @@ public void validate() {
         if (async && maxFrameDurationMs < 0) {
             throw new IllegalArgumentException("maxFrameDurationMs needs to be a non-negative number (it's how long JBWAPI waits for a bot response before returning control to BWAPI).");
         }
-        if (async && asyncFrameBufferSize < 1) {
-            throw new IllegalArgumentException("asyncFrameBufferSize needs to be a positive number (There needs to be at least one frame buffer).");
+        if (async && asyncFrameBufferCapacity < 1) {
+            throw new IllegalArgumentException("asyncFrameBufferCapacity needs to be a positive number (There needs to be at least one frame buffer).");
         }
     }
 
diff --git a/src/main/java/bwapi/BotWrapper.java b/src/main/java/bwapi/BotWrapper.java
@@ -11,12 +11,15 @@ class BotWrapper {
     private final BWClientConfiguration configuration;
     private final BWEventListener eventListener;
     private final FrameBuffer frameBuffer;
-    private Game game;
+    private ByteBuffer liveData;
+    private Game botGame;
     private Thread botThread;
     private boolean gameOver;
     private PerformanceMetrics performanceMetrics;
     private Throwable lastBotThrow;
     private ReentrantLock lastBotThrowLock = new ReentrantLock();
+    private ReentrantLock unsafeReadReadyLock = new ReentrantLock();
+    private boolean unsafeReadReady = false;
 
     BotWrapper(BWClientConfiguration configuration, BWEventListener eventListener) {
         this.configuration = configuration;
@@ -25,16 +28,17 @@ class BotWrapper {
     }
 
     /**
-     * Resets the BotWrapper for a new game.
+     * Resets the BotWrapper for a new botGame.
      */
     void startNewGame(ByteBuffer liveData, PerformanceMetrics performanceMetrics) {
         if (configuration.async) {
             frameBuffer.initialize(liveData, performanceMetrics);
         }
         this.performanceMetrics = performanceMetrics;
-        game = new Game();
-        game.clientData().setBuffer(liveData);
+        botGame = new Game();
+        botGame.clientData().setBuffer(liveData);
         liveClientData.setBuffer(liveData);
+        this.liveData = liveData;
         botThread = null;
         gameOver = false;
     }
@@ -44,7 +48,25 @@ void startNewGame(ByteBuffer liveData, PerformanceMetrics performanceMetrics) {
      * In asynchronous mode this Game object may point at a copy of a previous frame.
      */
     Game getGame() {
-        return game;
+        return botGame;
+    }
+
+    private boolean isUnsafeReadReady() {
+        unsafeReadReadyLock.lock();
+        try { return unsafeReadReady; }
+        finally { unsafeReadReadyLock.unlock(); }
+    }
+
+    private void setUnsafeReadReady(boolean value) {
+        unsafeReadReadyLock.lock();
+        try { unsafeReadReady = value; }
+        finally { unsafeReadReadyLock.unlock(); }
+        frameBuffer.lockSize.lock();
+        try {
+            frameBuffer.conditionSize.signalAll();
+        } finally {
+            frameBuffer.lockSize.unlock();
+        }
     }
 
     /**
@@ -61,17 +83,44 @@ void onFrame() {
                 botThread.setName("JBWAPI Bot");
                 botThread.start();
             }
-            /*
-            Add a frame to buffer
-            If buffer is full, it will wait until it has capacity
-            Wait for empty buffer OR termination condition
-            */
+
+            // Unsafe mode:
+            // If the frame buffer is empty (meaning the bot must be idle)
+            // allow the bot to read directly from shared memory while we copy it over
+            if (configuration.asyncUnsafe) {
+                frameBuffer.lockSize.lock();
+                try {
+                    if (frameBuffer.empty()) {
+                        configuration.log("Main: Putting bot on live data");
+                        botGame.clientData().setBuffer(liveData);
+                        setUnsafeReadReady(true);
+                    } else {
+                        setUnsafeReadReady(false);
+                    }
+                } finally {
+                    frameBuffer.lockSize.unlock();
+                }
+            }
+
+            // Add a frame to buffer
+            // If buffer is full, will wait until it has capacity.
+            // Then wait for the buffer to empty or to run out of time in the frame.
             int frame = liveClientData.gameData().getFrameCount();
             configuration.log("Main: Enqueuing frame #" + frame);
             frameBuffer.enqueueFrame();
+            configuration.log("Main: Enqueued frame #" + frame);
             frameBuffer.lockSize.lock();
             try {
                 while (!frameBuffer.empty()) {
+                    // Unsafe mode: Move the bot off of live data onto the frame buffer
+                    // This is the unsafe step!
+                    // We don't synchronize on calls which access the buffer
+                    // (to avoid tens of thousands of synchronized calls per frame)
+                    // so there's no guarantee of safety here.
+                    if (configuration.asyncUnsafe && frameBuffer.size() == 1) {
+                        configuration.log("Main: Weaning bot off live data");
+                        botGame.clientData().setBuffer(frameBuffer.peek());
+                    }
 
                     // Make bot exceptions fall through to the main thread.
                     Throwable lastThrow = getLastBotThrow();
@@ -130,26 +179,35 @@ private Thread createBotThread() {
                 configuration.log("Bot: Thread started");
                 while (!gameOver) {
 
+                    boolean doUnsafeRead = false;
                     configuration.log("Bot: Ready for another frame");
-                    performanceMetrics.botIdle.time(() -> {
-                        frameBuffer.lockSize.lock();
-                        try {
-                            while (frameBuffer.empty()) {
-                                configuration.log("Bot: Waiting for a frame");
-                                frameBuffer.conditionSize.awaitUninterruptibly();
-                            }
-                        } finally {
-                            frameBuffer.lockSize.unlock();
+                    performanceMetrics.botIdle.startTiming();
+                    frameBuffer.lockSize.lock();
+                    try {
+                        doUnsafeRead = isUnsafeReadReady();
+                        while ( ! doUnsafeRead && frameBuffer.empty()) {
+                            configuration.log("Bot: Waiting for a frame");
+                            frameBuffer.conditionSize.awaitUninterruptibly();
+                            doUnsafeRead = isUnsafeReadReady();
                         }
-                    });
+                    } finally {
+                        frameBuffer.lockSize.unlock();
+                    }
+                    performanceMetrics.botIdle.stopTiming();
 
-                    configuration.log("Bot: Peeking next frame");
-                    game.clientData().setBuffer(frameBuffer.peek());
+                    if (doUnsafeRead) {
+                        configuration.log("Bot: Reading live frame");
+                        setUnsafeReadReady(false);
+                        // TODO: Maybe we should point it at live data from here?
+                    } else {
+                        configuration.log("Bot: Peeking next frame from buffer");
+                        botGame.clientData().setBuffer(frameBuffer.peek());
+                    }
 
-                    configuration.log("Bot: Handling frame #" + game.getFrameCount());
+                    configuration.log("Bot: Handling events on frame #" + botGame.getFrameCount());
                     handleEvents();
 
-                    configuration.log("Bot: Events done. Dequeuing frame #" + game.getFrameCount());
+                    configuration.log("Bot: Events handled. Dequeuing frame #" + botGame.getFrameCount());
                     frameBuffer.dequeue();
                 }
             } catch (Throwable throwable) {
@@ -169,7 +227,7 @@ private Thread createBotThread() {
     }
 
     private void handleEvents() {
-        ClientData.GameData gameData = game.clientData().gameData();
+        ClientData.GameData gameData = botGame.clientData().gameData();
 
         // Populate gameOver before invoking event handlers (in case the bot throws)
         for (int i = 0; i < gameData.getEventCount(); i++) {
@@ -184,7 +242,7 @@ private void handleEvents() {
             ! gameOver && (gameData.getFrameCount() > 0 || ! configuration.unlimitedFrameZero),
             () -> {
                 for (int i = 0; i < gameData.getEventCount(); i++) {
-                    EventHandler.operation(eventListener, game, gameData.getEvents(i));
+                    EventHandler.operation(eventListener, botGame, gameData.getEvents(i));
                 }
             });
     }
diff --git a/src/main/java/bwapi/FrameBuffer.java b/src/main/java/bwapi/FrameBuffer.java
@@ -17,7 +17,7 @@ class FrameBuffer {
     private ByteBuffer liveData;
     private PerformanceMetrics performanceMetrics;
     private BWClientConfiguration configuration;
-    private int size;
+    private int capacity;
     private int stepGame = 0;
     private int stepBot = 0;
     private ArrayList<ByteBuffer> dataBuffer = new ArrayList<>();
@@ -27,9 +27,9 @@ class FrameBuffer {
     final Condition conditionSize = lockSize.newCondition();
 
     FrameBuffer(BWClientConfiguration configuration) {
-        this.size = configuration.asyncFrameBufferSize;
+        this.capacity = configuration.asyncFrameBufferCapacity;
         this.configuration = configuration;
-        while(dataBuffer.size() < size) {
+        while(dataBuffer.size() < capacity) {
             dataBuffer.add(ByteBuffer.allocateDirect(BUFFER_SIZE));
         }
     }
@@ -52,36 +52,43 @@ synchronized int framesBuffered() {
     }
 
     /**
-     * @return Whether the frame buffer is empty and has no frames available for the bot to consume.
+     * @return Number of frames currently stored in the buffer
      */
-    boolean empty() {
+    int size() {
         lockSize.lock();
         try {
-            return framesBuffered() <= 0;
+            return framesBuffered();
         }  finally {
             lockSize.unlock();
         }
     }
 
+    /**
+     * @return Whether the frame buffer is empty and has no frames available for the bot to consume.
+     */
+    boolean empty() {
+        return size() <= 0;
+    }
+
     /**
      * @return Whether the frame buffer is full and can not buffer any additional frames.
      * When the frame buffer is full, JBWAPI must wait for the bot to complete a frame before returning control to StarCraft.
      */
     boolean full() {
         lockSize.lock();
         try {
-            return framesBuffered() >= size;
+            return framesBuffered() >= capacity;
         } finally {
             lockSize.unlock();
         }
     }
 
     private int indexGame() {
-        return stepGame % size;
+        return stepGame % capacity;
     }
 
     private int indexBot() {
-        return stepBot % size;
+        return stepBot % capacity;
     }
 
     /**
diff --git a/src/main/java/bwapi/PerformanceMetrics.java b/src/main/java/bwapi/PerformanceMetrics.java
@@ -57,6 +57,18 @@ public class PerformanceMetrics {
      */
     public PerformanceMetric botIdle;
 
+    public PerformanceMetric frameDuration5;
+    public PerformanceMetric frameDuration10;
+    public PerformanceMetric frameDuration15;
+    public PerformanceMetric frameDuration20;
+    public PerformanceMetric frameDuration25;
+    public PerformanceMetric frameDuration30;
+    public PerformanceMetric frameDuration35;
+    public PerformanceMetric frameDuration40;
+    public PerformanceMetric frameDuration45;
+    public PerformanceMetric frameDuration50;
+    public PerformanceMetric frameDuration55;
+
     private BWClientConfiguration configuration;
 
     public PerformanceMetrics(BWClientConfiguration configuration) {
@@ -65,18 +77,28 @@ public PerformanceMetrics(BWClientConfiguration configuration) {
     }
 
     void reset() {
-        final int frameDurationBufferMs = 5;
         final int sideEffectsBufferMs = 1;
         final int realTimeFrameMs = 42;
-        totalFrameDuration = new PerformanceMetric("Total frame duration", configuration.maxFrameDurationMs + frameDurationBufferMs);
-        copyingToBuffer = new PerformanceMetric("Time copying to buffer", 15);
-        intentionallyBlocking = new PerformanceMetric("Intentionally blocking", 0);
+        totalFrameDuration = new PerformanceMetric("JBWAPI frame duration", configuration.maxFrameDurationMs + 5);
+        copyingToBuffer = new PerformanceMetric("Time copying to buffer", configuration.maxFrameDurationMs + 5);
+        intentionallyBlocking = new PerformanceMetric("Blocking with full buffer", 0);
         frameBufferSize = new PerformanceMetric("Frames buffered", 0);
-        framesBehind = new PerformanceMetric("Frames behind", 0);
-        flushSideEffects = new PerformanceMetric("Flush side effects", sideEffectsBufferMs );
-        botResponse = new PerformanceMetric("Bot responses", configuration.maxFrameDurationMs);
-        bwapiResponse = new PerformanceMetric("BWAPI responses", realTimeFrameMs);
+        framesBehind = new PerformanceMetric("Frames behind real-time", 0);
+        flushSideEffects = new PerformanceMetric("Flushing side effects", sideEffectsBufferMs );
+        botResponse = new PerformanceMetric("Bot event handlers", configuration.maxFrameDurationMs);
+        bwapiResponse = new PerformanceMetric("Responses from BWAPI", realTimeFrameMs);
         botIdle = new PerformanceMetric("Bot idle", Long.MAX_VALUE);
+        frameDuration5 = new PerformanceMetric("JBWAPI frame @  5ms", 5);
+        frameDuration10 = new PerformanceMetric("JBWAPI frame @ 10ms", 10);
+        frameDuration15 = new PerformanceMetric("JBWAPI frame @ 15ms", 15);
+        frameDuration20 = new PerformanceMetric("JBWAPI frame @ 20ms", 20);
+        frameDuration25 = new PerformanceMetric("JBWAPI frame @ 25ms", 25);
+        frameDuration30 = new PerformanceMetric("JBWAPI frame @ 30ms", 30);
+        frameDuration35 = new PerformanceMetric("JBWAPI frame @ 35ms", 35);
+        frameDuration40 = new PerformanceMetric("JBWAPI frame @ 40ms", 40);
+        frameDuration45 = new PerformanceMetric("JBWAPI frame @ 45ms", 45);
+        frameDuration50 = new PerformanceMetric("JBWAPI frame @ 50ms", 50);
+        frameDuration55 = new PerformanceMetric("JBWAPI frame @ 55ms", 55);
     }
 
     @Override
@@ -90,6 +112,17 @@ public String toString() {
                 + "\n" + flushSideEffects.toString()
                 + "\n" + botResponse.toString()
                 + "\n" + bwapiResponse.toString()
-                + "\n" + botIdle.toString();
+                + "\n" + botIdle.toString()
+                + "\n" + frameDuration5.toString()
+                + "\n" + frameDuration10.toString()
+                + "\n" + frameDuration15.toString()
+                + "\n" + frameDuration20.toString()
+                + "\n" + frameDuration25.toString()
+                + "\n" + frameDuration30.toString()
+                + "\n" + frameDuration35.toString()
+                + "\n" + frameDuration40.toString()
+                + "\n" + frameDuration45.toString()
+                + "\n" + frameDuration50.toString()
+                + "\n" + frameDuration55.toString();
     }
 }
diff --git a/src/test/java/bwapi/SynchronizationEnvironment.java b/src/test/java/bwapi/SynchronizationEnvironment.java
@@ -86,7 +86,7 @@ void runGame(int onEndFrame) {
         if (configuration.async) {
             final long MEGABYTE = 1024 * 1024;
             long memoryFree = Runtime.getRuntime().freeMemory() / MEGABYTE;
-            long memoryRequired = configuration.asyncFrameBufferSize * ClientData.GameData.SIZE / MEGABYTE;
+            long memoryRequired = configuration.asyncFrameBufferCapacity * ClientData.GameData.SIZE / MEGABYTE;
             assertTrue(
                     "Unit test needs to be run with sufficient memory to allocate frame buffer. Has "
                             + memoryFree
diff --git a/src/test/java/bwapi/SynchronizationTest.java b/src/test/java/bwapi/SynchronizationTest.java
