From 1e480b9626e0806ae9ff6f8f7ed616164d84ecb1 Mon Sep 17 00:00:00 2001 From: John Hurliman Date: Thu, 10 Sep 2026 00:13:35 -0700 Subject: [PATCH] Fix sketch key handling, result isolation and heap update performance --- .github/workflows/ci.yml | 17 +++++++++++++++ CHANGELOG.md | 14 +++++++++++++ bench/repeated-updates.js | 13 ++++++++++++ index.js | 2 +- lib/countMinSketch.js | 8 +++---- lib/hyperLogLog.js | 2 +- package.json | 26 ++++++++++++++++------- test/countminsketch-test.js | 6 +++--- test/hyperloglog-test.js | 6 +++--- test/regression-test.js | 42 +++++++++++++++++++++++++++++++++++++ test/runBatch.js | 11 ++++++++++ 11 files changed, 127 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 CHANGELOG.md create mode 100644 bench/repeated-updates.js create mode 100644 test/regression-test.js create mode 100644 test/runBatch.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..3813617 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,17 @@ +name: CI +on: [push, pull_request] +permissions: + contents: read +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + node: [22, 24, 26] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + - run: npm test + - run: npm pack diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..b7f7f74 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,14 @@ +# Changelog + +## Unreleased + +- Count keys such as `__proto__`, `constructor` and `toString` without interacting with Object.prototype. +- Return independent top-k tuples so callers cannot modify internal sketch state. +- Match serialized-size estimates to the actual power-of-two width. +- Repair only the affected heap path on tracked-key increments instead of sorting the entire heap. A local Node 24/macOS arm64 benchmark of 100,000 repeat updates across 1,000 tracked keys measured a seven-run median of 29 ms versus 1,141 ms before this change. This is workload-specific; see `bench/repeated-updates.js`. +- Restore the original test scenarios on Node's built-in runner, add regression coverage, and add current-Node CI and explicit package contents. +- Replace deprecated Buffer construction with zero-initialized buffers. + +### Release compatibility + +Runtime minimum becomes Node 6 because Buffer.alloc is now used; test development requires modern Node (CI: 22/24/26). Dropping previously advertised Node 0.6 support requires a major release. No version has been bumped yet. Top-k tie ordering is unspecified and may change. The existing binary format is retained; it does not record the original top-k capacity when a sketch is serialized before filling, so that capacity cannot be fully recovered. Malformed-input/deserialization validation remains a follow-up before a release is considered complete. diff --git a/bench/repeated-updates.js b/bench/repeated-updates.js new file mode 100644 index 0000000..ebb6d92 --- /dev/null +++ b/bench/repeated-updates.js @@ -0,0 +1,13 @@ +const path = require('node:path'); +const { performance } = require('node:perf_hooks'); +const CountMinSketch = require(path.resolve(process.argv[2] || path.join(__dirname, '../lib/countMinSketch.js'))); +const times = []; +for (let trial = 0; trial < 7; trial++) { + const sketch = new CountMinSketch(1000, 0.0005, 0.0001); + const keys = Array.from({length: 1000}, (_, i) => 'item-' + i); + keys.forEach(key => sketch.increment(key)); + const start = performance.now(); + for (let i = 0; i < 100000; i++) sketch.increment(keys[(i * 337) % 1000]); + times.push(performance.now() - start); +} +console.log(JSON.stringify({times, median: times.sort((a,b) => a-b)[3]})); diff --git a/index.js b/index.js index 82f4f5a..1eb944d 100644 --- a/index.js +++ b/index.js @@ -65,6 +65,6 @@ function getUniquesObjSize(stdError) { */ function getViewsObjSize(errFactor, failRate) { var depth = Math.max(Math.ceil(Math.log(1.0 / failRate)), 1); - var width = Math.ceil(Math.E / errFactor); + var width = Math.pow(2, Math.ceil(Math.log(Math.ceil(Math.E / errFactor)) / Math.LN2)); return 4 + 8 + depth * width * 4 + 4 + depth * 4 + 4; } diff --git a/lib/countMinSketch.js b/lib/countMinSketch.js index 8adcf22..f196a6b 100644 --- a/lib/countMinSketch.js +++ b/lib/countMinSketch.js @@ -30,7 +30,7 @@ var MAX_INT = 0xFFFFFFFF; function CountMinSketch(maxEntries, epsilon, delta, lgWidth, counts, hashFunctions, heap) { var i; var mapLen = 0; - var map = {}; + var map = Object.create(null); if (maxEntries) { // Depth of the 2D storage array. Equal to the number of hash functions @@ -138,7 +138,7 @@ function CountMinSketch(maxEntries, epsilon, delta, lgWidth, counts, hashFunctio } else { // Update the existing tuple and re-sort the priority queue probe[0] = est; - heapq.heap.sort(sortAsc); + heapq.heapify(heapq.heap.indexOf(probe)); } } @@ -151,7 +151,7 @@ function CountMinSketch(maxEntries, epsilon, delta, lgWidth, counts, hashFunctio */ function getTopK() { // Create a copy of the heap backing store - var vals = heapq.heap.slice(0); + var vals = heapq.heap.map(function(entry) { return entry.slice(); }); // Sort in descending order since the priority queue is sorted in ascending // order and only maintains partial ordering vals.sort(sortDesc); @@ -173,7 +173,7 @@ function CountMinSketch(maxEntries, epsilon, delta, lgWidth, counts, hashFunctio var depth = counts.length; var width = counts[0].length; - var buffer = new Buffer( + var buffer = Buffer.alloc( 4 + 8 + counts.length * width * 4 + 4 + hashFunctions.length * 4 + diff --git a/lib/hyperLogLog.js b/lib/hyperLogLog.js index 88ffd50..c3fb77d 100644 --- a/lib/hyperLogLog.js +++ b/lib/hyperLogLog.js @@ -102,7 +102,7 @@ function HyperLogLog(stdError, M, k_comp, alpha_m) { * structure. */ function serialize() { - var buffer = new Buffer(12 + this.M.length * 4); + var buffer = Buffer.alloc(12 + this.M.length * 4); buffer.writeUInt32LE(k_comp, 0, true); buffer.writeDoubleLE(alpha_m, 4, true); for (var i = 0; i < this.M.length; i++) diff --git a/package.json b/package.json index b6fbcc5..519fea3 100644 --- a/package.json +++ b/package.json @@ -3,22 +3,32 @@ "version": "1.0.1", "author": "John Hurliman (http://jhurliman.org/)", "description": "Provides implementations of sketch algorithms for real-time counting of stream data. Useful for real-time web analytics and other streaming or big data scenarios.", - "keywords": ["analytics", "metrics", "countminsketch", "hyperloglog", "sketch", "realtime"], + "keywords": [ + "analytics", + "metrics", + "countminsketch", + "hyperloglog", + "sketch", + "realtime" + ], "license": "MIT", "main": "./index", "scripts": { - "test": "vows --spec" - }, - "devDependencies": { - "assert": "1.3.0", - "vows": "0.8.1" + "test": "node --test test/*-test.js", + "bench": "node bench/repeated-updates.js" }, + "devDependencies": {}, "engines": { - "node": ">= 0.6.0" + "node": ">=6" }, "repository": { "type": "git", "url": "https://github.com/jhurliman/node-streamcount.git" }, - "homepage": "https://github.com/jhurliman/node-streamcount" + "homepage": "https://github.com/jhurliman/node-streamcount", + "files": [ + "index.js", + "lib/", + "CHANGELOG.md" + ] } diff --git a/test/countminsketch-test.js b/test/countminsketch-test.js index 442c513..1547832 100644 --- a/test/countminsketch-test.js +++ b/test/countminsketch-test.js @@ -1,9 +1,9 @@ -var vows = require('vows'); +var runBatch = require('./runBatch'); var assert = require('assert'); var CountMinSketch = require('../lib/countMinSketch'); -vows.describe('CountMinSketch').addBatch({ +runBatch('CountMinSketch', { '20 videos, 0.0005 epsilon, 0.0001 delta': { topic: new CountMinSketch(20, 0.0005, 0.0001), @@ -71,7 +71,7 @@ vows.describe('CountMinSketch').addBatch({ } }, }, -}).export(module); +}); function pad(number, length) { var str = '' + number; diff --git a/test/hyperloglog-test.js b/test/hyperloglog-test.js index 16cf38a..bdaa3e1 100644 --- a/test/hyperloglog-test.js +++ b/test/hyperloglog-test.js @@ -1,9 +1,9 @@ -var vows = require('vows'); +var runBatch = require('./runBatch'); var assert = require('assert'); var HyperLogLog = require('../lib/hyperLogLog'); -vows.describe('HyperLogLog').addBatch({ +runBatch('HyperLogLog', { '1% error rate': { topic: new HyperLogLog(0.01), @@ -107,7 +107,7 @@ vows.describe('HyperLogLog').addBatch({ assert.equal(hll.count(), hll2.count()); }, }, -}).export(module); +}); function pad(number, length) { var str = '' + number; diff --git a/test/regression-test.js b/test/regression-test.js new file mode 100644 index 0000000..5fe6974 --- /dev/null +++ b/test/regression-test.js @@ -0,0 +1,42 @@ +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { CountMinSketch, getViewsObjSize } = require('..'); + +test('counts keys which match Object prototype properties', () => { + const sketch = new CountMinSketch(10, 0.001, 0.001); + sketch.increment('toString'); + assert.deepEqual(sketch.getTopK(), [[1, 'toString']]); +}); +test('top-k results cannot mutate internal count or key tuples', () => { + const sketch = new CountMinSketch(10, 0.001, 0.001); + sketch.increment('a'); + const result = sketch.getTopK(); + result[0][0] = 100; + result[0][1] = 'changed'; + assert.deepEqual(sketch.getTopK(), [[1, 'a']]); +}); +test('size estimate uses the actual power-of-two sketch width', () => { + const sketch = new CountMinSketch(10, 0.002, 0.0001); + assert.equal(getViewsObjSize(0.002, 0.0001), sketch.serialize().length); +}); + +test('reserved keys survive serialization and later increments', () => { + const sketch = new CountMinSketch(10, 0.001, 0.001); + const keys = ['__proto__', 'constructor', 'toString']; + const previous = Object.getOwnPropertyDescriptor(Object.prototype, '0'); + for (const key of keys) sketch.increment(key); + const restored = CountMinSketch.deserialize(sketch.serialize()); + for (const key of keys) restored.increment(key); + assert.deepEqual(restored.getTopK().sort((a,b) => a[1].localeCompare(b[1])), keys.map(key => [2,key]).sort((a,b) => a[1].localeCompare(b[1]))); + assert.deepEqual(Object.getOwnPropertyDescriptor(Object.prototype, '0'), previous); +}); +test('heap repairs retain every tracked key and its count', () => { + const sketch = new CountMinSketch(1000, 0.0005, 0.0001); + const keys = Array.from({ length: 1000 }, (_, i) => 'item-' + i); + keys.forEach(key => sketch.increment(key)); + for (let i = 0; i < 100000; i++) sketch.increment(keys[(i * 337) % 1000]); + const result = sketch.getTopK(); + assert.equal(result.length, 1000); + assert.equal(new Set(result.map(entry => entry[1])).size, 1000); + assert.ok(result.every(entry => entry[0] === 101)); +}); diff --git a/test/runBatch.js b/test/runBatch.js new file mode 100644 index 0000000..1d8b75d --- /dev/null +++ b/test/runBatch.js @@ -0,0 +1,11 @@ +// Preserve the legacy suite's ordered, shared-topic assertions using node:test. +module.exports = function runBatch(name, groups) { + const { test } = require('node:test'); + test(name, async (t) => { + for (const [groupName, group] of Object.entries(groups)) { + for (const [assertion, run] of Object.entries(group)) { + if (assertion !== 'topic') await t.test(groupName + ': ' + assertion, () => run(group.topic)); + } + } + }); +};