remove parallel tests and replace memcpy with unsafe.copyMemory

Author: JasperGeurtz

pom.xml

@@ -60,12 +60,12 @@
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>
                 <version>2.22.0</version>
-                <configuration>
-                    <parallel>all</parallel> <!-- Run tests in parallel-->
-                    <useUnlimitedThreads>true</useUnlimitedThreads>
-                    <argLine>-Xms1g</argLine>
-                    <argLine>-Xmx1g</argLine>
-                </configuration>
+<!--                <configuration>-->
+<!--                    <parallel>all</parallel> &lt;!&ndash; Run tests in parallel&ndash;&gt;-->
+<!--                    <useUnlimitedThreads>true</useUnlimitedThreads>-->
+<!--                    <argLine>-Xms1g</argLine>-->
+<!--                    <argLine>-Xmx1g</argLine>-->
+<!--                </configuration>-->
             </plugin>
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>

src/main/java/bwapi/FrameBuffer.java

@@ -1,8 +1,7 @@
 package bwapi;
 
-import com.sun.jna.Platform;
-import sun.nio.ch.DirectBuffer;
-import java.nio.ByteBuffer;
+import sun.misc.Unsafe;
+
 import java.util.ArrayList;
 import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.Lock;
@@ -13,6 +12,7 @@
  */
 class FrameBuffer {
     private static final int BUFFER_SIZE = ClientData.GameData.SIZE;
+    private static final Unsafe unsafe = UnsafeTools.getUnsafe();
 
     private WrappedBuffer liveData;
     private PerformanceMetrics performanceMetrics;
@@ -158,24 +158,11 @@ void dequeue() {
      * @param source Address to copy from
      * @param destination Address to copy to
      * @param size Number of bytes to copy
-     * @return True if the copy succeeded
      */
-    private boolean tryMemcpyBuffer(WrappedBuffer source, WrappedBuffer destination, long offset, int size) {
+    private void copyBuffer(WrappedBuffer source, WrappedBuffer destination, long offset, int size) {
         long addressSource = source.getAddress() + offset;
         long addressDestination = destination.getAddress() + offset;
-        try {
-            if (Platform.isWindows()) {
-                if (Platform.is64Bit()) {
-                    MSVCRT.INSTANCE.memcpy(addressDestination, addressSource, size);
-                    return true;
-                } else {
-                    MSVCRT.INSTANCE.memcpy((int) addressDestination, (int) addressSource, size);
-                    return true;
-                }
-            }
-        }
-        catch(Exception ignored) {}
-        return false;
+        unsafe.copyMemory(addressSource, addressDestination, size);
     }
 
     void copyBuffer(WrappedBuffer source, WrappedBuffer destination, boolean copyEverything) {
@@ -185,16 +172,10 @@ void copyBuffer(WrappedBuffer source, WrappedBuffer destination, boolean copyEve
         but are prone to large amounts of variance.
 
         The normal Java way to execute this copy is via ByteBuffer.put(), which has reasonably good performance characteristics.
-        Experiments in 64-bit JRE have shown that using a native memcpy achieves a 35% speedup.
-        Experiments in 32-bit JRE show no difference in performance.
-
-        So, speculatively, we attempt to do a native memcpy.
         */
 
         if (copyEverything) {
-            if (tryMemcpyBuffer(source, destination, 0, FrameBuffer.BUFFER_SIZE)) {
-                return;
-            }
+            copyBuffer(source, destination, 0, FrameBuffer.BUFFER_SIZE);
         } else {
             // After the buffer has been filled the first time,
             // we can omit copying blocks of data which are unused or which don't change after game start.
@@ -207,18 +188,10 @@ void copyBuffer(WrappedBuffer source, WrappedBuffer destination, boolean copyEve
             final int STRINGSSHAPES_START = 10962632; // getStringCount, ... getShapes
             final int STRINGSHAPES_END = 32242636;
             final int UNITFINDER_START = 32962644;
-            if (
-                tryMemcpyBuffer(source, destination, 0, STATICTILES_START)
-                && tryMemcpyBuffer(source, destination, STATICTILES_END, REGION_START - STATICTILES_END)
-                && tryMemcpyBuffer(source, destination, REGION_END, STRINGSSHAPES_START - REGION_END)
-                && tryMemcpyBuffer(source, destination, STRINGSHAPES_END, UNITFINDER_START - STRINGSHAPES_END)) {
-                return;
-            }
+            copyBuffer(source, destination, 0, STATICTILES_START);
+            copyBuffer(source, destination, STATICTILES_END, REGION_START - STATICTILES_END);
+            copyBuffer(source, destination, REGION_END, STRINGSSHAPES_START - REGION_END);
+            copyBuffer(source, destination, STRINGSHAPES_END, UNITFINDER_START - STRINGSHAPES_END);
         }
-
-        // There's no specific case where we expect to fail above,
-        // but this is a safe fallback regardless,
-        // and serves to document the known-good (and cross-platform, for BWAPI 5) way to executing the copy.
-        destination.copyFrom(source);
     }
 }

src/main/java/bwapi/MSVCRT.java

@@ -0,0 +1,25 @@
+package bwapi;
+
+import sun.misc.Unsafe;
+
+import java.lang.reflect.Field;
+
+class UnsafeTools {
+    private static Unsafe unsafe;
+
+    static {
+        try {
+            final Field theUnsafe = Unsafe.class.getDeclaredField("theUnsafe");
+            theUnsafe.setAccessible(true);
+            unsafe = (Unsafe) theUnsafe.get(null);
+
+        } catch (final Exception e) {
+            e.printStackTrace();
+            System.exit(-1);
+        }
+    }
+
+    static Unsafe getUnsafe() {
+        return unsafe;
+    }
+}

src/main/java/bwapi/UnsafeTools.java

@@ -4,7 +4,6 @@
 import com.sun.jna.Pointer;
 import sun.misc.Unsafe;
 
-import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
 
 /**
@@ -14,19 +13,7 @@ class WrappedBuffer {
     private final ByteBuffer buffer;
     private final long address;
 
-    private static Unsafe unsafe;
-
-    static {
-        try {
-            final Field theUnsafe = Unsafe.class.getDeclaredField("theUnsafe");
-            theUnsafe.setAccessible(true);
-            unsafe = (Unsafe) theUnsafe.get(null);
-
-        } catch (final Exception e) {
-            e.printStackTrace();
-            System.exit(-1);
-        }
-    }
+    private static final Unsafe unsafe = UnsafeTools.getUnsafe();
 
     WrappedBuffer(final int size) {
         this(new Memory(size), size);
@@ -37,12 +24,6 @@ class WrappedBuffer {
         this.address = Pointer.nativeValue(pointer);
     }
 
-    void copyFrom(WrappedBuffer source) {
-        source.buffer.rewind();
-        buffer.rewind();
-        buffer.put(source.buffer);
-    }
-
     byte getByte(final int offset) {
         return unsafe.getByte(address + offset);
     }