Skip to content

Commit 437f09a

Browse files
committed
Iterative .entries & co (replacing recursive)
1.3× performance improvement, which is much more modest than all the previous, but we take those.
1 parent 8715eeb commit 437f09a

File tree

3 files changed

+76
-39
lines changed

3 files changed

+76
-39
lines changed

benchmark/entries.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
const benchmark = require('benchmark')
2+
const AKM = require('../main.js')
3+
4+
const letters = 'abcdefghijklmnopqrstuvwxyz'.split('')
5+
6+
const lettersMap = new AKM()
7+
for (const x of letters) lettersMap.set(Array(100).fill(x), true)
8+
9+
new benchmark.Suite()
10+
.add('100-item iterator', () => {
11+
const entriesGenerator = lettersMap.entries()
12+
while (!entriesGenerator.next().done) { continue }
13+
})
14+
.on('cycle', ev => console.log(String(ev.target)))
15+
.run()

main.js

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,15 @@ class ArrayKeyedMap {
8282

8383
get [Symbol.toStringTag] () { return 'ArrayKeyedMap' }
8484

85-
* [Symbol.iterator] () { yield * entries([], this._root) }
85+
* [Symbol.iterator] () { yield * entries.call(this) }
8686

87-
* entries () { yield * entries([], this._root) }
87+
* entries () { yield * entries.call(this) }
8888

89-
* keys () { yield * keys(this._root) }
89+
* keys () { yield * keys.call(this) }
9090

91-
* values () { yield * values(this._root) }
91+
* values () { yield * values.call(this) }
9292

93-
forEach (callback, thisArg) { forEach(this._root, callback, thisArg, this) }
93+
forEach (callback, thisArg) { forEach.call(this, callback, thisArg) }
9494
}
9595

9696
module.exports = ArrayKeyedMap
@@ -184,23 +184,25 @@ function hasPrefix (path) {
184184
return true
185185
}
186186

187-
const entries = function * (path, store) {
188-
for (const [key, value] of store) {
189-
if (key === dataSymbol) yield [path, value]
190-
else {
191-
yield * entries(path.concat([key]), value)
187+
function * entries () {
188+
const stack = [{ path: [], map: this._root }]
189+
while (stack.length > 0) {
190+
const { path, map } = stack.pop()
191+
for (const [k, v] of map.entries()) {
192+
if (k === dataSymbol) yield [path, v]
193+
else stack.push({ path: path.concat([k]), map: v })
192194
}
193195
}
194196
}
195197

196-
const keys = function * (store) {
197-
for (const [k] of entries([], store)) yield k
198+
function * keys () {
199+
for (const [k] of this.entries()) yield k
198200
}
199201

200-
const values = function * (store) {
201-
for (const [, v] of entries([], store)) yield v
202+
function * values () {
203+
for (const [, v] of this.entries()) yield v
202204
}
203205

204-
const forEach = (store, callback, thisArg, main) => {
205-
for (const [k, v] of entries([], store)) callback.call(thisArg, v, k, main)
206+
function forEach (callback, thisArg) {
207+
for (const [k, v] of this.entries()) callback.call(thisArg, v, k, this)
206208
}

test.js

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const test = require('tape')
22
const AKM = require('./main.js')
3+
const assert = require('assert')
34

45
test('empty', (t) => {
56
const p = new AKM()
@@ -244,41 +245,60 @@ test('iterators', (t) => {
244245
const value4 = 'ba'
245246
p.set(key4, value4)
246247

247-
// Note that entries 3 and 4 come in the opposite order in every iterator.
248-
// This is OK, because this module doesn't guarantee iteration order.
248+
// We don't guarantee any particular order in which entries are emitted by
249+
// iterators. So to make this test work even if the implementation changes,
250+
// we specifically disregard order when checking them.
251+
function containsAllOf (t, array, expectedItems) {
252+
t.same(
253+
array.length,
254+
expectedItems.length,
255+
'Iterator has right number of elements')
256+
257+
expectedItems.forEach(expectedItem => {
258+
const wasFound = array.some(actualItem => {
259+
try {
260+
assert.deepStrictEqual(actualItem, expectedItem)
261+
} catch (e) {
262+
if (e instanceof assert.AssertionError) return false
263+
else throw e
264+
}
265+
return true
266+
})
267+
t.ok(wasFound, `Iterator contains ${JSON.stringify(expectedItem)}`)
268+
})
269+
}
270+
249271
test('entries', (t) => {
250-
const iterator = p.entries()
251-
t.same(iterator.next().value, [key1, value1])
252-
t.same(iterator.next().value, [key2, value2])
253-
t.same(iterator.next().value, [key4, value4])
254-
t.same(iterator.next().value, [key3, value3])
272+
containsAllOf(t, [...p.entries()], [
273+
[key1, value1],
274+
[key2, value2],
275+
[key3, value3],
276+
[key4, value4]
277+
])
255278
t.end()
256279
})
257280

258281
test('@@iterator', (t) => {
259-
const a = Array.from(p)
260-
t.same(a[0], [key1, value1])
261-
t.same(a[1], [key2, value2])
262-
t.same(a[2], [key4, value4])
263-
t.same(a[3], [key3, value3])
282+
containsAllOf(t, Array.from(p), [
283+
[key1, value1],
284+
[key2, value2],
285+
[key3, value3],
286+
[key4, value4]
287+
])
264288
t.end()
265289
})
266290

267291
test('keys', (t) => {
268-
const iterator = p.keys()
269-
t.same(iterator.next().value, key1)
270-
t.same(iterator.next().value, key2)
271-
t.same(iterator.next().value, key4)
272-
t.same(iterator.next().value, key3)
292+
containsAllOf(t, [...p.keys()], [
293+
key1, key2, key3, key4
294+
])
273295
t.end()
274296
})
275297

276298
test('values', (t) => {
277-
const iterator = p.values()
278-
t.same(iterator.next().value, value1)
279-
t.same(iterator.next().value, value2)
280-
t.same(iterator.next().value, value4)
281-
t.same(iterator.next().value, value3)
299+
containsAllOf(t, [...p.values()], [
300+
value1, value2, value3, value4
301+
])
282302
t.end()
283303
})
284304

@@ -291,7 +311,7 @@ test('iterators', (t) => {
291311
},
292312
thisValue)
293313
t.same(forEachReturnValue, undefined)
294-
t.same(kvs, [
314+
containsAllOf(t, kvs, [
295315
{ thisValue: thisValue, args: [value1, key1, p] },
296316
{ thisValue: thisValue, args: [value2, key2, p] },
297317
{ thisValue: thisValue, args: [value4, key4, p] },

0 commit comments

Comments
 (0)