From 944ce1dcca37df8ccac977fdba4e7a29a005b461 Mon Sep 17 00:00:00 2001 From: Young Hahn Date: Sun, 29 Jan 2017 17:41:01 -0500 Subject: [PATCH 1/4] Add unit test to demonstrate bitmask overflow => allocation fail --- test/coalesce.test.js | 99 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/test/coalesce.test.js b/test/coalesce.test.js index e22bedd..b28a7f9 100644 --- a/test/coalesce.test.js +++ b/test/coalesce.test.js @@ -690,3 +690,102 @@ test('coalesce args', function(assert) { }); }); })(); + +// Mask overflow +(function() { + var a = new Cache('a', 0); + var b = new Cache('b', 0); + var c = new Cache('c', 0); + + var grids = []; + for (var i = 1; i < 50e3; i++) grids.push(Grid.encode({ + id: i, + x: 0, + y: 0, + relev: 1, + score: 1 + })); + a._set('grid', 0, 1, grids); + + b._set('grid', 0, 1, [ + Grid.encode({ + id: 1, + x: 0, + y: 0, + relev: 1, + score: 1 + }) + ]); + c._set('grid', 0, 1, [ + Grid.encode({ + id: 1, + x: 0, + y: 0, + relev: 1, + score: 1 + }) + ]); + + test('coalesceMulti mask safe', function(assert) { + assert.comment('start coalesce (mask: 2)'); + coalesce([{ + cache: a, + mask: 1 << 2, + idx: 0, + zoom: 0, + weight: 0.33, + phrase: 1 + }, { + cache: b, + mask: 1 << 0, + idx: 1, + zoom: 0, + weight: 0.33, + phrase: 1 + }, { + cache: c, + mask: 1 << 1, + idx: 2, + zoom: 0, + weight: 0.33, + phrase: 1 + }], {}, function(err, res) { + assert.ifError(err); + assert.equal(res.length, 1, 'res length = 1'); + assert.deepEqual(res[0].map(function(f) { return f.id; }), [1, 1, 1], '0.relev = 0.99'); + assert.end(); + }); + }); + + test('coalesceMulti mask overflow', function(assert) { + assert.comment('start coalesce (mask: 18)'); + coalesce([{ + cache: a, + mask: 1 << 18, + idx: 0, + zoom: 0, + weight: 0.33, + phrase: 1 + }, { + cache: b, + mask: 1 << 0, + idx: 1, + zoom: 0, + weight: 0.33, + phrase: 1 + }, { + cache: c, + mask: 1 << 1, + idx: 2, + zoom: 0, + weight: 0.33, + phrase: 1 + }], {}, function(err, res) { + assert.ifError(err); + assert.equal(res.length, 1, 'res length = 1'); + assert.deepEqual(res[0].map(function(f) { return f.id; }), [1, 1, 1], '0.relev = 0.99'); + assert.end(); + }); + }); +})(); + From f634f865798c8365cba1e0cafb5731e18cd6bf0a Mon Sep 17 00:00:00 2001 From: Young Hahn Date: Sun, 29 Jan 2017 17:47:11 -0500 Subject: [PATCH 2/4] Test blows up with just 10k grids, go with that --- test/coalesce.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/coalesce.test.js b/test/coalesce.test.js index b28a7f9..4ae52d5 100644 --- a/test/coalesce.test.js +++ b/test/coalesce.test.js @@ -698,7 +698,7 @@ test('coalesce args', function(assert) { var c = new Cache('c', 0); var grids = []; - for (var i = 1; i < 50e3; i++) grids.push(Grid.encode({ + for (var i = 1; i < 10e3; i++) grids.push(Grid.encode({ id: i, x: 0, y: 0, From 754ada73c5017c5fd457a4fc982cc3e593d9be8f Mon Sep 17 00:00:00 2001 From: Young Hahn Date: Sun, 29 Jan 2017 17:52:34 -0500 Subject: [PATCH 3/4] Increase bitspace for mask from unsigned short (16 bits) to uint32_t (32 bits) to handle carmen's 20 token limit --- src/binding.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/binding.cpp b/src/binding.cpp index 5a87597..b5ae397 100644 --- a/src/binding.cpp +++ b/src/binding.cpp @@ -840,7 +840,7 @@ struct PhrasematchSubq { uint64_t phrase; unsigned short idx; unsigned short zoom; - unsigned short mask; + uint32_t mask; }; struct Cover { @@ -851,19 +851,19 @@ struct Cover { unsigned short y; unsigned short score; unsigned short idx; - unsigned short mask; + uint32_t mask; unsigned distance; double scoredist; }; struct Context { std::vector coverList; - unsigned mask; + uint32_t mask; double relev; Context(Context const& c) = default; Context(Cover && cov, - unsigned mask, + uint32_t mask, double relev) : coverList(), mask(mask), @@ -877,7 +877,7 @@ struct Context { return *this; } Context(std::vector && cl, - unsigned mask, + uint32_t mask, double relev) : coverList(std::move(cl)), mask(mask), @@ -1187,8 +1187,7 @@ void coalesceSingle(uv_work_t* req) { added++; double relev = cover.relev; - // TODO correct default mask value? - unsigned mask = 0; + uint32_t mask = 0; contexts.emplace_back(std::move(cover),mask,relev); } @@ -1312,7 +1311,7 @@ void coalesceMulti(uv_work_t* req) { std::vector covers; covers.reserve(stackSize); covers.push_back(cover); - unsigned context_mask = cover.mask; + uint32_t context_mask = cover.mask; double context_relev = cover.relev; for (unsigned a = 0; a < zCacheSize; a++) { @@ -1323,7 +1322,7 @@ void coalesceMulti(uv_work_t* req) { static_cast(std::floor(cover.y/s)); pit = coalesced.find(pxy); if (pit != coalesced.end()) { - unsigned lastMask = 0; + uint32_t lastMask = 0; double lastRelev = 0.0; for (auto const& parents : pit->second) { for (auto const& parent : parents.coverList) { @@ -1475,7 +1474,7 @@ NAN_METHOD(Cache::coalesce) { subq.weight = jsStack->Get(Nan::New("weight").ToLocalChecked())->NumberValue(); subq.phrase = jsStack->Get(Nan::New("phrase").ToLocalChecked())->IntegerValue(); - subq.mask = static_cast(jsStack->Get(Nan::New("mask").ToLocalChecked())->IntegerValue()); + subq.mask = static_cast(jsStack->Get(Nan::New("mask").ToLocalChecked())->IntegerValue()); // JS cache reference => cpp Local cache = Local::Cast(jsStack->Get(Nan::New("cache").ToLocalChecked())); From 90e593e86bc90c8d02d31d8e1fc76c325f2bce47 Mon Sep 17 00:00:00 2001 From: Young Hahn Date: Sun, 29 Jan 2017 19:49:53 -0500 Subject: [PATCH 4/4] Update tape ht @mattficke --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 950c75e..df3c080 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "protozero": "~1.4.5" }, "devDependencies": { - "tape": "3.0.x" + "tape": "^4.6.3" }, "main": "./index.js", "scripts": {