From bc3c8d401755ce14055987c784b79b915c56bd74 Mon Sep 17 00:00:00 2001 From: ferrinweb Date: Thu, 30 Sep 2021 13:22:50 +0800 Subject: [PATCH 1/8] Add support for multiple pMapSkips --- .gitignore | 1 + index.js | 5 ++++- test.js | 26 ++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 239ecff..d7a57fa 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ node_modules yarn.lock +.history diff --git a/index.js b/index.js index c7c9fba..c0718ab 100644 --- a/index.js +++ b/index.js @@ -62,6 +62,9 @@ export default async function pMap( } else { isResolved = true; + // Delete skipped index from back to front, to support multiple pMapSkips + // so use the unshift method when putting index into skippedIndexes. + // see line 93 for (const skippedIndex of skippedIndexes) { result.splice(skippedIndex, 1); } @@ -87,7 +90,7 @@ export default async function pMap( const value = await mapper(element, index); if (value === pMapSkip) { - skippedIndexes.push(index); + skippedIndexes.unshift(index); } else { result[index] = value; } diff --git a/test.js b/test.js index 2120258..4c45472 100644 --- a/test.js +++ b/test.js @@ -155,6 +155,19 @@ test('pMapSkip', async t => { ], async value => value), [1, 2]); }); +test('multiple pMapSkips', async t => { + t.deepEqual(await pMap([ + 1, + pMapSkip, + 2, + pMapSkip, + 3, + pMapSkip, + pMapSkip, + 4 + ], async value => value), [1, 2, 3, 4]); +}); + test('all mappers should run when concurrency is infinite, even after stop-on-error happened', async t => { const input = [1, async () => delay(300, {value: 2}), 3]; const mappedValues = []; @@ -269,6 +282,19 @@ test('asyncIterator - pMapSkip', async t => { ]), async value => value), [1, 2]); }); +test('asyncIterator - multiple pMapSkips', async t => { + t.deepEqual(await pMap(new AsyncTestData([ + 1, + pMapSkip, + 2, + pMapSkip, + 3, + pMapSkip, + pMapSkip, + 4 + ]), async value => value), [1, 2, 3, 4]); +}); + test('asyncIterator - all mappers should run when concurrency is infinite, even after stop-on-error happened', async t => { const input = [1, async () => delay(300, {value: 2}), 3]; const mappedValues = []; From a58e786376bfe63562318a3337168620617d4cb1 Mon Sep 17 00:00:00 2001 From: ferrinweb Date: Thu, 30 Sep 2021 13:39:17 +0800 Subject: [PATCH 2/8] remove .history rule from .gitignore --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index d7a57fa..239ecff 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,2 @@ node_modules yarn.lock -.history From ef13ee409a40a7ff6b5cdf806b523fe330371eb5 Mon Sep 17 00:00:00 2001 From: ferrinweb Date: Tue, 5 Oct 2021 11:21:41 +0800 Subject: [PATCH 3/8] multiple pMapSkips performance improvement --- index.js | 36 ++++++++++++++++--------- test-multiple-pmapskips-performance.js | 37 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 test-multiple-pmapskips-performance.js diff --git a/index.js b/index.js index c0718ab..36dc499 100644 --- a/index.js +++ b/index.js @@ -23,7 +23,7 @@ export default async function pMap( const result = []; const errors = []; - const skippedIndexes = []; + const skippedIndexesMap = new Map(); let isRejected = false; let isResolved = false; let isIterableDone = false; @@ -59,18 +59,29 @@ export default async function pMap( if (resolvingCount === 0 && !isResolved) { if (!stopOnError && errors.length > 0) { reject(new AggregateError(errors)); - } else { - isResolved = true; + return; + } - // Delete skipped index from back to front, to support multiple pMapSkips - // so use the unshift method when putting index into skippedIndexes. - // see line 93 - for (const skippedIndex of skippedIndexes) { - result.splice(skippedIndex, 1); - } + isResolved = true; + if (!skippedIndexesMap.size) { resolve(result); + return; } + + const pureResult = []; + + // Support multiple pMapSkips + // eslint-disable-next-line unicorn/no-for-loop + for (let index = 0; index < result.length; index++) { + if (skippedIndexesMap.get(index) === pMapSkip) { + continue; + } + + pureResult.push(result[index]); + } + + resolve(pureResult); } return; @@ -89,12 +100,13 @@ export default async function pMap( const value = await mapper(element, index); + // Use Map to stage the index of the element. if (value === pMapSkip) { - skippedIndexes.unshift(index); - } else { - result[index] = value; + skippedIndexesMap.set(index, value); } + result[index] = value; + resolvingCount--; await next(); } catch (error) { diff --git a/test-multiple-pmapskips-performance.js b/test-multiple-pmapskips-performance.js new file mode 100644 index 0000000..9745748 --- /dev/null +++ b/test-multiple-pmapskips-performance.js @@ -0,0 +1,37 @@ +import test from 'ava'; +import inRange from 'in-range'; +import timeSpan from 'time-span'; +import pMap, {pMapSkip} from './index.js'; + +function generateSkipPerformanceData(length) { + const data = []; + for (let index = 0; index < length; index++) { + data.push(pMapSkip); + } + + return data; +} + +test('multiple pMapSkips - algorithmic complexity', async t => { + const testData = [generateSkipPerformanceData(1000), generateSkipPerformanceData(10000), generateSkipPerformanceData(100000)]; + const testDurationsMS = []; + + for (const data of testData) { + const end = timeSpan(); + // eslint-disable-next-line no-await-in-loop + await pMap(data, async value => value); + testDurationsMS.push(end()); + } + + for (let index = 0; index < testDurationsMS.length - 1; index++) { + // Time for 10x more items should take between 9x and 11x more time + const smallerDuration = testDurationsMS[index]; + const longerDuration = testDurationsMS[index + 1]; + + // The longer test needs to be a little longer and also not 10x more than the + // shorter test. This is not perfect... there is some fluctuation. + // The idea here is to catch a regression that makes pMapSkip handling O(n^2) + // on the number of pMapSkip items in the input + t.true(inRange(longerDuration, {start: 1.2 * smallerDuration, end: 15 * smallerDuration})); + } +}); From a0244534ec999348146bff7a85df0c42f7b4989c Mon Sep 17 00:00:00 2001 From: ferrinweb Date: Tue, 5 Oct 2021 11:40:49 +0800 Subject: [PATCH 4/8] Add an all pMapSkips test --- test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test.js b/test.js index 4c45472..02a98d8 100644 --- a/test.js +++ b/test.js @@ -168,6 +168,15 @@ test('multiple pMapSkips', async t => { ], async value => value), [1, 2, 3, 4]); }); +test('all pMapSkips', async t => { + t.deepEqual(await pMap([ + pMapSkip, + pMapSkip, + pMapSkip, + pMapSkip + ], async value => value), []); +}); + test('all mappers should run when concurrency is infinite, even after stop-on-error happened', async t => { const input = [1, async () => delay(300, {value: 2}), 3]; const mappedValues = []; @@ -295,6 +304,15 @@ test('asyncIterator - multiple pMapSkips', async t => { ]), async value => value), [1, 2, 3, 4]); }); +test('asyncIterator - all pMapSkips', async t => { + t.deepEqual(await pMap(new AsyncTestData([ + pMapSkip, + pMapSkip, + pMapSkip, + pMapSkip + ]), async value => value), []); +}); + test('asyncIterator - all mappers should run when concurrency is infinite, even after stop-on-error happened', async t => { const input = [1, async () => delay(300, {value: 2}), 3]; const mappedValues = []; From cc97e462e2bcd85340a3389b5c089db05bbf4e91 Mon Sep 17 00:00:00 2001 From: ferrinweb Date: Wed, 27 Oct 2021 17:18:34 +0800 Subject: [PATCH 5/8] Follow the 'unicorn/no-for-loop' rule --- index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 36dc499..3cd1023 100644 --- a/index.js +++ b/index.js @@ -72,8 +72,8 @@ export default async function pMap( const pureResult = []; // Support multiple pMapSkips - // eslint-disable-next-line unicorn/no-for-loop - for (let index = 0; index < result.length; index++) { + const resultIterator = result.keys(); + for (const index of resultIterator) { if (skippedIndexesMap.get(index) === pMapSkip) { continue; } From 0dead6034db754504243c5cbf8d24d4e203054e9 Mon Sep 17 00:00:00 2001 From: ferrinweb Date: Wed, 27 Oct 2021 18:01:02 +0800 Subject: [PATCH 6/8] Optimize the result set traversal process Co-authored-by: Sindre Sorhus --- index.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 3cd1023..ab9caf9 100644 --- a/index.js +++ b/index.js @@ -72,13 +72,12 @@ export default async function pMap( const pureResult = []; // Support multiple pMapSkips - const resultIterator = result.keys(); - for (const index of resultIterator) { + for (const [index, value] of result.entries()) { if (skippedIndexesMap.get(index) === pMapSkip) { continue; } - pureResult.push(result[index]); + pureResult.push(value); } resolve(pureResult); From 9a6c0c6ec835af480a7957f5bc59b95bb015a870 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Tue, 2 Nov 2021 19:00:57 +0700 Subject: [PATCH 7/8] Update index.js --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index ab9caf9..11cf5f5 100644 --- a/index.js +++ b/index.js @@ -71,7 +71,7 @@ export default async function pMap( const pureResult = []; - // Support multiple pMapSkips + // Support multiple `pMapSkip`'s. for (const [index, value] of result.entries()) { if (skippedIndexesMap.get(index) === pMapSkip) { continue; From 63f1f01743eef79c70d6b93046ed3f2d8e003e88 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Tue, 2 Nov 2021 19:01:42 +0700 Subject: [PATCH 8/8] Update test-multiple-pmapskips-performance.js --- test-multiple-pmapskips-performance.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test-multiple-pmapskips-performance.js b/test-multiple-pmapskips-performance.js index 9745748..204b768 100644 --- a/test-multiple-pmapskips-performance.js +++ b/test-multiple-pmapskips-performance.js @@ -24,14 +24,14 @@ test('multiple pMapSkips - algorithmic complexity', async t => { } for (let index = 0; index < testDurationsMS.length - 1; index++) { - // Time for 10x more items should take between 9x and 11x more time + // Time for 10x more items should take between 9x and 11x more time. const smallerDuration = testDurationsMS[index]; const longerDuration = testDurationsMS[index + 1]; // The longer test needs to be a little longer and also not 10x more than the - // shorter test. This is not perfect... there is some fluctuation. - // The idea here is to catch a regression that makes pMapSkip handling O(n^2) - // on the number of pMapSkip items in the input + // shorter test. This is not perfect... there is some fluctuation. + // The idea here is to catch a regression that makes `pMapSkip` handling O(n^2) + // on the number of `pMapSkip` items in the input. t.true(inRange(longerDuration, {start: 1.2 * smallerDuration, end: 15 * smallerDuration})); } });