Skip to content

Commit 605ee67

Browse files
godsflawgodsflaw
committed
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.
1 parent 0b512b1 commit 605ee67

7 files changed

Lines changed: 233 additions & 77 deletions

File tree

‎README.md‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,31 +110,40 @@ stdin and writing to stdout.
110110
% echo '[1, 2, 3]' | ./bin/json2msgpack | ./bin/msgpack2json
111111
[1,2,3]
112112

113-
### Building and installation
113+
### Building, Installation, Testing
114114

115115
There are two ways to install msgpack.
116116

117-
## npm
117+
## NPM
118118

119119
npm install msgpack
120120

121121
This should build and install msgpack for you. Then just `require('msgpack')`.
122122

123123
## Manually
124124

125-
Use `make` to build the add-on, then manually copy `build/default/mpBindings.node`
126-
and `lib/msgpack.js` it to wherever your node.js installation will look for it (or
127-
add the build directory to your `$NODE_PATH`).
125+
You will need node-gyp:
126+
npm install -g node-gyp
128127

129-
% ls
130-
LICENSE Makefile README.md deps/ src/ tags test.js
131-
% make
128+
Then from the root of the msgpack repo, you can run:
129+
node-gyp rebuild
132130

133-
The MessagePack library on which this depends is packaged with `node-msgpack`
134-
and will be built as part of this process.
131+
NOTE: node-gyp attempts to contact the Internet and download the target version
132+
of node.js source and store it locally. This will only happen once for
133+
each time it sees a new node.js version. If you're on a host with no
134+
direct Internet access, you may need to shuffle this source over from
135+
another box or sneaker net. Good luck!
135136

136-
**Note:** MessagePack may fail to build if you do not have a modern version of
137-
gcc in your `$PATH`. On Mac OS X Snow Leopard (10.5.x), you may have to use
138-
`gcc-4.2`, which should come with your box but is not used by default.
137+
## Testing and Benchmarks
139138

140-
% make CC=gcc-4.2 CXX=gcc-4.2
139+
To run all tests use:
140+
./run_tests
141+
142+
To run a specific test:
143+
./run_tests test/lib/msgpack.js
144+
145+
To run benchmarks:
146+
./run_tests test/benchmark.js
147+
148+
NOTE: Tests are based on a modified version of nodeunit. Follow ./run_tests
149+
instructions if you run into problems.

‎bench.js‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎binding.gyp‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,30 @@
1919
'-Wall',
2020
'-O3'
2121
],
22-
'cflags!': ['-fno-exceptions'],
23-
'cflags_cc!': ['-fno-exceptions'],
22+
'cflags!': [
23+
'-fno-exceptions',
24+
'-Wno-unused-function'
25+
],
26+
'cflags_cc!': [
27+
'-fno-exceptions',
28+
'-Wno-unused-function'
29+
],
2430
'conditions': [
2531
['OS=="mac"', {
26-
'xcode_settings': {
27-
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES'
28-
}
29-
32+
'configurations': {
33+
'Debug': {
34+
'xcode_settings': {
35+
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
36+
'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
37+
}
38+
},
39+
'Release': {
40+
'xcode_settings': {
41+
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
42+
'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
43+
},
44+
},
45+
},
3046
}],
3147
['OS=="win"', {
3248
'configurations': {

‎deps/msgpack/msgpack.gyp‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,38 @@
1313
'version.c'
1414
],
1515
'cflags_cc': [
16-
'-Wall',
16+
'-all',
1717
'-O3'
1818
],
1919
'cflags': [
2020
'-Wall',
2121
'-O3'
2222
],
23-
'cflags!': ['-fno-exceptions'],
24-
'cflags_cc!': ['-fno-exceptions'],
23+
'cflags!': [
24+
'-fno-exceptions',
25+
'-Wno-unused-function'
26+
],
27+
'cflags_cc!': [
28+
'-fno-exceptions',
29+
'-Wno-unused-function'
30+
],
2531
'conditions': [
26-
['OS=="mac"', {
27-
'xcode_settings': {
28-
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES'
29-
}
30-
31-
}],
32+
['OS=="mac"', {
33+
'configurations': {
34+
'Debug': {
35+
'xcode_settings': {
36+
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
37+
'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
38+
}
39+
},
40+
'Release': {
41+
'xcode_settings': {
42+
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
43+
'WARNING_CFLAGS': ['-Wall', '-Wno-unused-function'],
44+
},
45+
},
46+
},
47+
}],
3248
['OS=="win"', {
3349
'configurations': {
3450
'Debug': {

‎src/msgpack.cc‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ class MsgpackSbuffer {
5151
}
5252

5353
~MsgpackSbuffer() {
54-
msgpack_sbuffer_destroy(&this->_sbuf);
54+
// godsflaw: No longer call msgpack_sbuffer_destroy here, as the
55+
// memory from _sbuf will be freed in the _free_sbuf callback,
56+
// which is called by the Buffer destructor. While this is more
57+
// complicated, it should yield a decent performance increase.
58+
msgpack_sbuffer_release(&this->_sbuf);
5559
}
5660
};
5761

@@ -74,6 +78,16 @@ class MsgpackSbuffer {
7478
} \
7579
} while (0)
7680

81+
// This will be passed to Buffer::New so that we can manage our own memory.
82+
// In other news, I am unsure what to do with hint, as I've never seen this
83+
// coding pattern before.
84+
static void
85+
_free_sbuf(char *data, void *hint) {
86+
if (data != NULL) {
87+
free(data);
88+
}
89+
}
90+
7791
// Convert a V8 object to a MessagePack object.
7892
//
7993
// This method is recursive. It will probably blow out the stack on objects
@@ -229,7 +243,7 @@ msgpack_to_v8(msgpack_object *mo) {
229243
// will be accumulated to the end of the previous value(s).
230244
//
231245
// Any number of objects can be provided as arguments, and all will be
232-
// serialized to the same bytestream, back-ty-back.
246+
// serialized to the same bytestream, back-to-back.
233247
static Handle<Value>
234248
pack(const Arguments &args) {
235249
HandleScope scope;
@@ -244,7 +258,7 @@ pack(const Arguments &args) {
244258
msgpack_object mo;
245259

246260
try {
247-
v8_to_msgpack(args[0], &mo, &mz._mz, 0);
261+
v8_to_msgpack(args[i], &mo, &mz._mz, 0);
248262
} catch (MsgpackException e) {
249263
return ThrowException(e.getThrownException());
250264
}
@@ -255,10 +269,24 @@ pack(const Arguments &args) {
255269
}
256270
}
257271

258-
Buffer *bp = Buffer::New(sb._sbuf.size);
259-
memcpy(Buffer::Data(bp), sb._sbuf.data, sb._sbuf.size);
272+
v8::Local<Buffer> slowBuffer = node::Buffer::New(
273+
sb._sbuf.data, sb._sbuf.size, _free_sbuf, 0
274+
);
260275

261-
return scope.Close(bp->handle_);
276+
v8::Local<Object> global = v8::Context::GetCurrent()->Global();
277+
v8::Local<Value> bv = global->Get(String::NewSymbol("Buffer"));
278+
279+
assert(bv->IsFunction());
280+
281+
Local<Function> bc = v8::Local<Function>::Cast(bv);
282+
Handle<Value> cArgs[3] = {
283+
slowBuffer->handle_,
284+
v8::Integer::New(sb._sbuf.size),
285+
v8::Integer::New(0)
286+
};
287+
v8::Local<Object> fastBuffer = bc->NewInstance(3, cArgs);
288+
289+
return scope.Close(fastBuffer);
262290
}
263291

264292
// var o = msgpack.unpack(buf);

‎test/benchmark.js‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
var fs = require('fs'),
2+
sys = require('sys'),
3+
msgpack = require("../lib/msgpack"),
4+
stub = require("./fixtures/stub");
5+
6+
var DATA_TEMPLATE = {'abcdef' : 1, 'qqq' : 13, '19' : [1, 2, 3, 4]};
7+
var DATA = [];
8+
9+
for (var i = 0; i < 500000; i++) {
10+
DATA.push(JSON.parse(JSON.stringify(DATA_TEMPLATE)));
11+
}
12+
13+
function _set_up(callback) {
14+
this.backup = {};
15+
callback();
16+
}
17+
18+
function _tear_down(callback) {
19+
callback();
20+
}
21+
22+
exports.benchmark = {
23+
setUp : _set_up,
24+
tearDown : _tear_down,
25+
'JSON.stringify no more than 7x faster than msgpack.pack' : function (test) {
26+
var jsonStr;
27+
var now = Date.now();
28+
DATA.forEach(function(d) {
29+
jsonStr = JSON.stringify(d);
30+
});
31+
var stringifyTime = (Date.now() - now);
32+
33+
var mpBuf;
34+
now = Date.now();
35+
DATA.forEach(function(d) {
36+
mpBuf = msgpack.pack(d);
37+
});
38+
var packTime = (Date.now() - now);
39+
40+
console.log(
41+
"msgpack.pack: "+packTime+"ms, JSON.stringify: "+stringifyTime+"ms"
42+
);
43+
console.log(
44+
"ratio of JSON.stringify/msgpack.pack: " + packTime/stringifyTime
45+
);
46+
test.expect(1);
47+
test.ok(
48+
packTime/stringifyTime < 7,
49+
"msgpack.pack: "+packTime+"ms, JSON.stringify: "+stringifyTime+"ms"
50+
);
51+
test.done();
52+
},
53+
'JSON.parse no more than 5x faster than msgpack.unpack' : function (test) {
54+
var jsonStr;
55+
DATA.forEach(function(d) {
56+
jsonStr = JSON.stringify(d);
57+
});
58+
59+
var mpBuf;
60+
DATA.forEach(function(d) {
61+
mpBuf = msgpack.pack(d);
62+
});
63+
64+
var now = Date.now();
65+
DATA.forEach(function(d) {
66+
JSON.parse(jsonStr);
67+
});
68+
var parseTime = (Date.now() - now);
69+
70+
now = Date.now();
71+
DATA.forEach(function(d) {
72+
msgpack.unpack(mpBuf);
73+
});
74+
var unpackTime = (Date.now() - now);
75+
76+
console.log(
77+
"msgpack.unpack: "+unpackTime+"ms, JSON.parse: "+parseTime+"ms"
78+
);
79+
console.log("ratio of JSON.parse/msgpack.unpack: " + unpackTime/parseTime);
80+
test.expect(1);
81+
test.ok(
82+
unpackTime/parseTime < 5,
83+
"msgpack.unpack: "+unpackTime+"ms, JSON.parse: "+parseTime+"ms"
84+
);
85+
test.done();
86+
},
87+
'output above is from three runs of benchmarks' : function (test) {
88+
console.log();
89+
for (var i = 0; i < 3; i++) {
90+
var mpBuf;
91+
var now = Date.now();
92+
DATA.forEach(function(d) {
93+
mpBuf = msgpack.pack(d);
94+
});
95+
console.log('msgpack pack: ' + (Date.now() - now) + ' ms');
96+
97+
now = Date.now();
98+
DATA.forEach(function(d) {
99+
msgpack.unpack(mpBuf);
100+
});
101+
console.log('msgpack unpack: ' + (Date.now() - now) + ' ms');
102+
103+
var jsonStr;
104+
now = Date.now();
105+
DATA.forEach(function(d) {
106+
jsonStr = JSON.stringify(d);
107+
});
108+
console.log('json pack: ' + (Date.now() - now) + ' ms');
109+
110+
now = Date.now();
111+
DATA.forEach(function(d) {
112+
JSON.parse(jsonStr);
113+
});
114+
console.log('json unpack: ' + (Date.now() - now) + ' ms');
115+
console.log();
116+
}
117+
118+
test.expect(1);
119+
test.ok(1);
120+
test.done();
121+
}
122+
};

‎test/lib/msgpack.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ exports.msgpack = {
3232
test.isObject(msgpack);
3333
test.done();
3434
},
35+
'pack should return a Buffer object' : function (test) {
36+
test.expect(2);
37+
var buf = msgpack.pack('abcdef');
38+
test.isNotNull(buf);
39+
test.isBuffer(buf);
40+
test.done();
41+
},
3542
'test for string equality' : function (test) {
3643
test.expect(1);
3744
test.deepEqual('abcdef', msgpack.unpack(msgpack.pack('abcdef')));

0 commit comments

Comments
 (0)