Bunch of updates

These are a bunch of updates to tests, benchmarks, packing,
and building.  This commit works, which is a good place to commit,
however, I made pack return a Buffer rather than a SlowBuffer, which
made packing even slower.  I am now working on optimizing pack and
unpack.

README.md

@@ -110,31 +110,40 @@ stdin and writing to stdout.
     % echo '[1, 2, 3]' | ./bin/json2msgpack | ./bin/msgpack2json 
     [1,2,3]
 
-### Building and installation
+### Building, Installation, Testing
 
 There are two ways to install msgpack.
 
-## npm
+## NPM
 
 		npm install msgpack
 
 This should build and install msgpack for you. Then just `require('msgpack')`.
 
 ## Manually
 
-Use `make` to build the add-on, then manually copy `build/default/mpBindings.node` 
-and `lib/msgpack.js` it to wherever your node.js installation will look for it (or
-add the build directory to your `$NODE_PATH`).
+You will need node-gyp:
+    npm install -g node-gyp
 
-    % ls
-    LICENSE  Makefile  README.md  deps/  src/  tags  test.js
-    % make
+Then from the root of the msgpack repo, you can run:
+    node-gyp rebuild
 
-The MessagePack library on which this depends is packaged with `node-msgpack`
-and will be built as part of this process.
+NOTE: node-gyp attempts to contact the Internet and download the target version
+      of node.js source and store it locally.  This will only happen once for
+      each time it sees a new node.js version.  If you're on a host with no
+      direct Internet access, you may need to shuffle this source over from
+      another box or sneaker net.  Good luck!
 
-**Note:** MessagePack may fail to build if you do not have a modern version of
-gcc in your `$PATH`. On Mac OS X Snow Leopard (10.5.x), you may have to use
-`gcc-4.2`, which should come with your box but is not used by default.
+## Testing and Benchmarks
 
-    % make CC=gcc-4.2 CXX=gcc-4.2
+To run all tests use:
+    ./run_tests
+
+To run a specific test:
+    ./run_tests test/lib/msgpack.js 
+
+To run benchmarks:
+    ./run_tests test/benchmark.js 
+
+NOTE: Tests are based on a modified version of nodeunit.  Follow ./run_tests
+      instructions if you run into problems.

binding.gyp

@@ -19,14 +19,30 @@
 					'-Wall',
 					'-O3'
 				],
-				'cflags!': ['-fno-exceptions'],
-				'cflags_cc!': ['-fno-exceptions'],
+				'cflags!': [
+          '-fno-exceptions',
+          '-Wno-unused-function'
+        ],
+				'cflags_cc!': [
+          '-fno-exceptions',
+          '-Wno-unused-function'
+        ],
 				'conditions': [
 					['OS=="mac"', {
-						'xcode_settings': {
-							'GCC_ENABLE_CPP_EXCEPTIONS': 'YES'
-						}
-
+						'configurations': {
+							'Debug': {
+						    'xcode_settings': {
+						    	'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
+                  'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
+						    }
+              },
+							'Release': {
+						    'xcode_settings': {
+						    	'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
+                  'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
+						    },
+              },
+            },
 					}],
 					['OS=="win"', {
 						'configurations': {

deps/msgpack/msgpack.gyp

@@ -13,22 +13,38 @@
 					'version.c'					
 				],
 				'cflags_cc': [
-					'-Wall',
+					'-all',
 					'-O3'
 				],
 				'cflags': [
 					'-Wall',
 					'-O3'
 				],
-				'cflags!': ['-fno-exceptions'],
-				'cflags_cc!': ['-fno-exceptions'],
+				'cflags!': [
+          '-fno-exceptions',
+          '-Wno-unused-function'
+        ],
+				'cflags_cc!': [
+          '-fno-exceptions',
+          '-Wno-unused-function'
+        ],
 				'conditions': [
-					['OS=="mac"', {
-						'xcode_settings': {
-							'GCC_ENABLE_CPP_EXCEPTIONS': 'YES'
-						}
-
-					}],
+          ['OS=="mac"', {
+            'configurations': {                                                 
+              'Debug': {       
+                'xcode_settings': {
+                  'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
+                  'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
+                }
+              },
+              'Release': {
+                'xcode_settings': {
+                  'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
+                  'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
+                },
+              },
+            },
+          }],
 					['OS=="win"', {
 						'configurations': {
 							'Debug': {

src/msgpack.cc

@@ -51,7 +51,11 @@ class MsgpackSbuffer {
         }
 
         ~MsgpackSbuffer() {
-            msgpack_sbuffer_destroy(&this->_sbuf);
+            // godsflaw: No longer call msgpack_sbuffer_destroy here, as the
+            // memory from _sbuf will be freed in the _free_sbuf callback,
+            // which is called by the Buffer destructor.  While this is more
+            // complicated, it should yield a decent performance increase.
+            msgpack_sbuffer_release(&this->_sbuf);
         }
 };
 
@@ -74,6 +78,16 @@ class MsgpackSbuffer {
         } \
     } while (0)
 
+// This will be passed to Buffer::New so that we can manage our own memory.
+// In other news, I am unsure what to do with hint, as I've never seen this
+// coding pattern before.
+static void
+_free_sbuf(char *data, void *hint) {
+    if (data != NULL) {
+        free(data);
+    }
+}
+
 // Convert a V8 object to a MessagePack object.
 //
 // This method is recursive. It will probably blow out the stack on objects
@@ -229,7 +243,7 @@ msgpack_to_v8(msgpack_object *mo) {
 // will be accumulated to the end of the previous value(s).
 //
 // Any number of objects can be provided as arguments, and all will be
-// serialized to the same bytestream, back-ty-back.
+// serialized to the same bytestream, back-to-back.
 static Handle<Value>
 pack(const Arguments &args) {
     HandleScope scope;
@@ -244,7 +258,7 @@ pack(const Arguments &args) {
         msgpack_object mo;
 
         try {
-            v8_to_msgpack(args[0], &mo, &mz._mz, 0);
+            v8_to_msgpack(args[i], &mo, &mz._mz, 0);
         } catch (MsgpackException e) {
             return ThrowException(e.getThrownException());
         }
@@ -255,10 +269,24 @@ pack(const Arguments &args) {
         }
     }
 
-    Buffer *bp = Buffer::New(sb._sbuf.size);
-    memcpy(Buffer::Data(bp), sb._sbuf.data, sb._sbuf.size);
+    v8::Local<Buffer> slowBuffer = node::Buffer::New(
+        sb._sbuf.data, sb._sbuf.size, _free_sbuf, 0
+    );
 
-    return scope.Close(bp->handle_);
+    v8::Local<Object> global = v8::Context::GetCurrent()->Global();
+    v8::Local<Value> bv = global->Get(String::NewSymbol("Buffer"));
+
+    assert(bv->IsFunction());
+
+    Local<Function> bc = v8::Local<Function>::Cast(bv);
+    Handle<Value> cArgs[3] = {
+        slowBuffer->handle_,
+        v8::Integer::New(sb._sbuf.size),
+        v8::Integer::New(0)
+    };
+    v8::Local<Object> fastBuffer = bc->NewInstance(3, cArgs);
+
+    return scope.Close(fastBuffer);
 }
 
 // var o = msgpack.unpack(buf);

test/benchmark.js

@@ -0,0 +1,122 @@
+var fs      = require('fs'),
+    sys     = require('sys'),
+    msgpack = require("../lib/msgpack"),
+    stub    = require("./fixtures/stub");
+
+var DATA_TEMPLATE = {'abcdef' : 1, 'qqq' : 13, '19' : [1, 2, 3, 4]};
+var DATA = [];
+
+for (var i = 0; i < 500000; i++) {
+    DATA.push(JSON.parse(JSON.stringify(DATA_TEMPLATE)));
+}
+
+function _set_up(callback) {
+  this.backup = {};
+  callback();
+}
+
+function _tear_down(callback) {
+  callback();
+}
+
+exports.benchmark = {
+  setUp : _set_up,
+  tearDown : _tear_down,
+  'JSON.stringify no more than 7x faster than msgpack.pack' : function (test) {
+    var jsonStr;
+    var now = Date.now();
+    DATA.forEach(function(d) {
+        jsonStr = JSON.stringify(d);
+    });
+    var stringifyTime = (Date.now() - now);
+
+    var mpBuf;
+    now = Date.now();
+    DATA.forEach(function(d) {
+        mpBuf = msgpack.pack(d);
+    });
+    var packTime = (Date.now() - now);
+
+    console.log(
+      "msgpack.pack: "+packTime+"ms, JSON.stringify: "+stringifyTime+"ms"
+    );
+    console.log(
+      "ratio of JSON.stringify/msgpack.pack: " + packTime/stringifyTime
+    );
+    test.expect(1);
+    test.ok(
+      packTime/stringifyTime < 7,
+      "msgpack.pack: "+packTime+"ms, JSON.stringify: "+stringifyTime+"ms"
+    );
+    test.done();
+  },
+  'JSON.parse no more than 5x faster than msgpack.unpack' : function (test) {
+    var jsonStr;
+    DATA.forEach(function(d) {
+        jsonStr = JSON.stringify(d);
+    });
+
+    var mpBuf;
+    DATA.forEach(function(d) {
+        mpBuf = msgpack.pack(d);
+    });
+
+    var now = Date.now();
+    DATA.forEach(function(d) {
+        JSON.parse(jsonStr);
+    });
+    var parseTime = (Date.now() - now);
+
+    now = Date.now();
+    DATA.forEach(function(d) {
+        msgpack.unpack(mpBuf);
+    });
+    var unpackTime = (Date.now() - now);
+
+    console.log(
+      "msgpack.unpack: "+unpackTime+"ms, JSON.parse: "+parseTime+"ms"
+    );
+    console.log("ratio of JSON.parse/msgpack.unpack: " + unpackTime/parseTime);
+    test.expect(1);
+    test.ok(
+      unpackTime/parseTime < 5,
+      "msgpack.unpack: "+unpackTime+"ms, JSON.parse: "+parseTime+"ms"
+    );
+    test.done();
+  },
+  'output above is from three runs of benchmarks' : function (test) {
+    console.log();
+    for (var i = 0; i < 3; i++) {
+      var mpBuf;
+      var now = Date.now();
+      DATA.forEach(function(d) {
+        mpBuf = msgpack.pack(d);
+      });
+      console.log('msgpack pack:   ' + (Date.now() - now) + ' ms');
+
+      now = Date.now();
+      DATA.forEach(function(d) {
+        msgpack.unpack(mpBuf);
+      });
+      console.log('msgpack unpack: ' + (Date.now() - now) + ' ms');
+
+      var jsonStr;
+      now = Date.now();
+      DATA.forEach(function(d) {
+        jsonStr = JSON.stringify(d);
+      });
+      console.log('json    pack:   ' + (Date.now() - now) + ' ms');
+
+      now = Date.now();
+      DATA.forEach(function(d) {
+        JSON.parse(jsonStr);
+      });
+      console.log('json    unpack: ' + (Date.now() - now) + ' ms');
+      console.log();
+    }
+
+    test.expect(1);
+    test.ok(1);
+    test.done();
+  }
+};

test/lib/msgpack.js

@@ -32,6 +32,13 @@ exports.msgpack = {
     test.isObject(msgpack);
     test.done();
   },
+  'pack should return a Buffer object' : function (test) {
+    test.expect(2);
+    var buf = msgpack.pack('abcdef');
+    test.isNotNull(buf);
+    test.isBuffer(buf);
+    test.done();
+  },
   'test for string equality' : function (test) {
     test.expect(1);
     test.deepEqual('abcdef', msgpack.unpack(msgpack.pack('abcdef')));