Skip to content

Commit 2dae8b0

Browse files
authored
feat(sdk): Mapping concurrency (#2899)
Improved concurrency for `Mapping` class. Now concurrent equal `get()` calls share the same promise, i.e. they evaluate the `valueFactory` only once. ## Example ```ts const valueFactory = async () => ... const mapping = new Mapping(valueFactory) await Promise.all([ mapping.get('foo'), mapping.get('bar'), mapping.get('bar'), mapping.get('foo'), mapping.get('foo') ]) ``` Triggers two calls to `valueFactory` (one for `foo` and another for `bar`). Before this PR it triggered 5 calls. ## Small refactoring - Added `readonly` field annotations - Updated deprecated `jest` method call
1 parent 882b884 commit 2dae8b0

File tree

2 files changed

+65
-9
lines changed

2 files changed

+65
-9
lines changed

packages/sdk/src/utils/Mapping.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,35 @@ interface ValueWrapper<V> {
1313
*/
1414
export class Mapping<K extends (string | number)[], V> {
1515

16-
private delegate: Map<string, ValueWrapper<V>> = new Map()
17-
private valueFactory: (...args: K) => Promise<V>
16+
private readonly delegate: Map<string, ValueWrapper<V>> = new Map()
17+
private readonly pendingPromises: Map<string, Promise<V>> = new Map()
18+
private readonly valueFactory: (...args: K) => Promise<V>
1819

1920
constructor(valueFactory: (...args: K) => Promise<V>) {
2021
this.valueFactory = valueFactory
2122
}
2223

2324
async get(...args: K): Promise<V> {
2425
const key = formLookupKey(...args)
25-
let valueWrapper = this.delegate.get(key)
26-
if (valueWrapper === undefined) {
27-
const value = await this.valueFactory(...args)
28-
valueWrapper = { value }
29-
this.delegate.set(key, valueWrapper)
26+
const pendingPromise = this.pendingPromises.get(key)
27+
if (pendingPromise !== undefined) {
28+
return await pendingPromise
29+
} else {
30+
let valueWrapper = this.delegate.get(key)
31+
if (valueWrapper === undefined) {
32+
const promise = this.valueFactory(...args)
33+
this.pendingPromises.set(key, promise)
34+
let value
35+
try {
36+
value = await promise
37+
} finally {
38+
this.pendingPromises.delete(key)
39+
}
40+
valueWrapper = { value }
41+
this.delegate.set(key, valueWrapper)
42+
}
43+
return valueWrapper.value
3044
}
31-
return valueWrapper.value
3245
}
3346

3447
values(): V[] {

packages/sdk/test/unit/Mapping.test.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { wait } from '@streamr/utils'
12
import { Mapping } from '../../src/utils/Mapping'
23

34
describe('Mapping', () => {
@@ -27,7 +28,49 @@ describe('Mapping', () => {
2728
const mapping = new Mapping(valueFactory)
2829
expect(await mapping.get('foo')).toBe(undefined)
2930
expect(await mapping.get('foo')).toBe(undefined)
30-
expect(valueFactory).toBeCalledTimes(1)
31+
expect(valueFactory).toHaveBeenCalledTimes(1)
3132
})
3233

34+
it('rejections are not cached', async () => {
35+
const valueFactory = jest.fn().mockImplementation(async (p1: string, p2: number) => {
36+
throw new Error(`error ${p1}-${p2}`)
37+
})
38+
const mapping = new Mapping(valueFactory)
39+
await expect(mapping.get('foo', 1)).rejects.toEqual(new Error('error foo-1'))
40+
await expect(mapping.get('foo', 1)).rejects.toEqual(new Error('error foo-1'))
41+
expect(valueFactory).toHaveBeenCalledTimes(2)
42+
})
43+
44+
it('throws are not cached', async () => {
45+
const valueFactory = jest.fn().mockImplementation((p1: string, p2: number) => {
46+
throw new Error(`error ${p1}-${p2}`)
47+
})
48+
const mapping = new Mapping(valueFactory)
49+
await expect(mapping.get('foo', 1)).rejects.toEqual(new Error('error foo-1'))
50+
await expect(mapping.get('foo', 1)).rejects.toEqual(new Error('error foo-1'))
51+
expect(valueFactory).toHaveBeenCalledTimes(2)
52+
})
53+
54+
it('concurrency', async () => {
55+
const valueFactory = jest.fn().mockImplementation(async (p1: string, p2: number) => {
56+
await wait(50)
57+
return `${p1}${p2}`
58+
})
59+
const mapping = new Mapping(valueFactory)
60+
const results = await Promise.all([
61+
mapping.get('foo', 1),
62+
mapping.get('foo', 2),
63+
mapping.get('foo', 2),
64+
mapping.get('foo', 1),
65+
mapping.get('foo', 1)
66+
])
67+
expect(valueFactory).toHaveBeenCalledTimes(2)
68+
expect(results).toEqual([
69+
'foo1',
70+
'foo2',
71+
'foo2',
72+
'foo1',
73+
'foo1'
74+
])
75+
})
3376
})

0 commit comments

Comments
 (0)