Merge pull request #380 from miniway/directbuf

Refactor MessageBuffer instantiation and add more tests

Author: xerial

msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java

@@ -198,7 +198,7 @@ public static MessageBuffer allocate(int size)
     }
 
     /**
-     * Wraps a ByteBuffer into a MessageBuffer.
+     * Wraps a byte array into a MessageBuffer.
      *
      * The new MessageBuffer will be backed by the given byte array. Modifications to the new MessageBuffer will cause the byte array to be modified and vice versa.
      *
@@ -214,7 +214,7 @@ public static MessageBuffer wrap(byte[] array)
     }
 
     /**
-     * Wraps a ByteBuffer into a MessageBuffer.
+     * Wraps a byte array into a MessageBuffer.
      *
      * The new MessageBuffer will be backed by the given byte array. Modifications to the new MessageBuffer will cause the byte array to be modified and vice versa.
      *
@@ -264,28 +264,7 @@ public static MessageBuffer wrap(ByteBuffer bb)
     private static MessageBuffer newMessageBuffer(byte[] arr, int off, int len)
     {
         checkNotNull(arr);
-        try {
-            return (MessageBuffer) mbArrConstructor.newInstance(arr, off, len);
-        }
-        catch (InstantiationException e) {
-            // should never happen
-            throw new IllegalStateException(e);
-        }
-        catch (IllegalAccessException e) {
-            // should never happen unless security manager restricts this reflection
-            throw new IllegalStateException(e);
-        }
-        catch (InvocationTargetException e) {
-            if (e.getCause() instanceof RuntimeException) {
-                // underlaying constructor may throw RuntimeException
-                throw (RuntimeException) e.getCause();
-            }
-            else if (e.getCause() instanceof Error) {
-                throw (Error) e.getCause();
-            }
-            // should never happen
-            throw new IllegalStateException(e.getCause());
-        }
+        return newInstance(mbArrConstructor, arr, off, len);
     }
 
     /**
@@ -297,10 +276,21 @@ else if (e.getCause() instanceof Error) {
     private static MessageBuffer newMessageBuffer(ByteBuffer bb)
     {
         checkNotNull(bb);
+        return newInstance(mbBBConstructor, bb);
+    }
+
+    /**
+     * Creates a new MessageBuffer instance
+     *
+     * @param constructor A MessageBuffer constructor
+     * @return new MessageBuffer instance
+     */
+    private static MessageBuffer newInstance(Constructor<?> constructor, Object... args)
+    {
         try {
             // We need to use reflection to create MessageBuffer instances in order to prevent TypeProfile generation for getInt method. TypeProfile will be
             // generated to resolve one of the method references when two or more classes overrides the method.
-            return (MessageBuffer) mbBBConstructor.newInstance(bb);
+            return (MessageBuffer) constructor.newInstance(args);
         }
         catch (InstantiationException e) {
             // should never happen

msgpack-core/src/test/scala/org/msgpack/core/MessageUnpackerTest.scala

@@ -345,6 +345,22 @@ class MessageUnpackerTest extends MessagePackSpec {
 
     "be faster then msgpack-v6 skip" taggedAs ("cmp-skip") in {
 
+      trait Fixture {
+        val unpacker: MessageUnpacker
+        def run {
+          var count = 0
+          try {
+            while (unpacker.hasNext) {
+              unpacker.skipValue()
+              count += 1
+            }
+          }
+          finally {
+            unpacker.close()
+          }
+        }
+      }
+
       val data = testData3(10000)
       val N = 100
 
@@ -367,21 +383,29 @@ class MessageUnpackerTest extends MessagePackSpec {
             unpacker.close()
         }
 
-        block("v7") {
-          val unpacker = Random.shuffle(unpackers(data)).head
-          var count = 0
-          try {
-            while (unpacker.hasNext) {
-              unpacker.skipValue()
-              count += 1
-            }
-          }
-          finally
-            unpacker.close()
+        block("v7-array") {
+          new Fixture { override val unpacker = MessagePack.newDefaultUnpacker(data) }.run
+        }
+
+        block("v7-array-buffer") {
+          new Fixture {
+            val bb = ByteBuffer.allocate(data.length)
+            bb.put(data).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(bb)
+          }.run
+        }
+        block("v7-direct-buffer") {
+          new Fixture {
+            val db = ByteBuffer.allocateDirect(data.length)
+            db.put(data).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(db)
+          }.run
         }
       }
 
-      t("v7").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
+      t("v7-array").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
+      t("v7-array-buffer").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
+      t("v7-direct-buffer").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
     }
 
     import org.msgpack.`type`.{ValueType => ValueTypeV6}
@@ -448,6 +472,20 @@ class MessageUnpackerTest extends MessagePackSpec {
             unpacker.skipValue()
         }
       }
+      trait Fixture {
+        val unpacker : MessageUnpacker
+        def run {
+          var count = 0
+          try {
+            while (unpacker.hasNext) {
+              readValue(unpacker)
+              count += 1
+            }
+          }
+          finally
+            unpacker.close()
+        }
+      }
 
       val data = testData3(10000)
       val N = 100
@@ -469,22 +507,29 @@ class MessageUnpackerTest extends MessagePackSpec {
             unpacker.close()
         }
 
-        block("v7") {
-          val unpacker = Random.shuffle(unpackers(data)).head
-          var count = 0
-          try {
-            while (unpacker.hasNext) {
-              readValue(unpacker)
-              count += 1
-            }
-          }
-          finally
-            unpacker.close()
+        block("v7-array") {
+          new Fixture { override val unpacker = MessagePack.newDefaultUnpacker(data) }.run
         }
-      }
 
-      t("v7").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
+        block("v7-array-buffer") {
+          new Fixture {
+            val bb = ByteBuffer.allocate(data.length)
+            bb.put(data).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(bb)
+          }.run
+        }
+        block("v7-direct-buffer") {
+          new Fixture {
+            val db = ByteBuffer.allocateDirect(data.length)
+            db.put(data).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(db)
+          }.run
+        }
+      }
 
+      t("v7-array").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
+      t("v7-array-buffer").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
+      t("v7-direct-buffer").averageWithoutMinMax should be <= t("v6").averageWithoutMinMax
 
     }
 
@@ -500,6 +545,35 @@ class MessageUnpackerTest extends MessagePackSpec {
       }
       packer.close()
 
+      trait Fixture {
+        val unpacker : MessageUnpacker
+        val loop : Int
+        def run {
+          var i = 0
+          try {
+            while (i < loop) {
+              val len = unpacker.unpackBinaryHeader()
+              val out = new Array[Byte](len)
+              unpacker.readPayload(out, 0, len)
+              i += 1
+            }
+          }
+          finally
+            unpacker.close()
+        }
+        def runRef {
+          var i = 0
+          try {
+            while (i < loop) {
+              val len = unpacker.unpackBinaryHeader()
+              val out = unpacker.readPayloadAsReference(len)
+              i += 1
+            }
+          }
+          finally
+            unpacker.close()
+        }
+      }
       val b = bos.toByteArray
       time("unpackBinary", repeat = 100) {
         block("v6") {
@@ -513,27 +587,54 @@ class MessageUnpackerTest extends MessagePackSpec {
           unpacker.close()
         }
 
-        block("v7") {
-          val unpacker = Random.shuffle(unpackers(b)).head
-          var i = 0
-          while (i < R) {
-            val len = unpacker.unpackBinaryHeader()
-            val out = new Array[Byte](len)
-            unpacker.readPayload(out, 0, len)
-            i += 1
-          }
-          unpacker.close()
+        block("v7-array") {
+          new Fixture {
+            override val unpacker = MessagePack.newDefaultUnpacker(b)
+            override val loop = R
+          }.run
         }
 
-        block("v7-ref") {
-          val unpacker = Random.shuffle(unpackers(b)).head
-          var i = 0
-          while (i < R) {
-            val len = unpacker.unpackBinaryHeader()
-            val out = unpacker.readPayloadAsReference(len)
-            i += 1
-          }
-          unpacker.close()
+        block("v7-array-buffer") {
+          new Fixture {
+            val bb = ByteBuffer.allocate(b.length)
+            bb.put(b).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(bb)
+            override val loop = R
+          }.run
+        }
+
+        block("v7-direct-buffer") {
+          new Fixture {
+            val db = ByteBuffer.allocateDirect(b.length)
+            db.put(b).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(db)
+            override val loop = R
+          }.run
+        }
+
+        block("v7-ref-array") {
+          new Fixture {
+            override val unpacker = MessagePack.newDefaultUnpacker(b)
+            override val loop = R
+          }.run
+        }
+
+        block("v7-ref-array-buffer") {
+          new Fixture {
+            val bb = ByteBuffer.allocate(b.length)
+            bb.put(b).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(bb)
+            override val loop = R
+          }.run
+        }
+
+        block("v7-ref-direct-buffer") {
+          new Fixture {
+            val db = ByteBuffer.allocateDirect(b.length)
+            db.put(b).flip()
+            override val unpacker = MessagePack.newDefaultUnpacker(db)
+            override val loop = R
+          }.run
         }
       }
     }