From ea042458a3f378f1cb83545b3291f3552092bf85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= <9092381+Renegade334@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:03:26 +0000 Subject: [PATCH 1/2] perf(collection): optimisations (#10552) * perf: `merge()`: deduplicate boolean checks * perf: `toSorted()`: remove redundant closure * perf: `last[Key]()`: order of operations - do not perform iterable-to-array until required - test ! before < * perf: `{at,keyAt}()`: manually iterate to target * perf: `first[Key]()`: avoid `Array.from()` * perf: `map()`: avoid `Array.from()` * perf: `random[Key]()`: avoid `Array.from()` * test: `.{at,keyAt}()` indices * perf: `last[Key]()`: use `.at()`/`.keyAt()` for single element * perf: `first[Key]()`: use iterable-to-array if returning all * perf: `random[Key]()`: use `{at,keyAt}()` for single value - skip iterable-to-array for returning single value - short-circuit if amount or collection size is zero * perf: `random[Key]()`: use Durstenfeld shuffle * refactor: `{key,keyAt}()`: reorder index check --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../collection/__tests__/collection.test.ts | 20 ++- packages/collection/src/collection.ts | 142 ++++++++++++------ 2 files changed, 115 insertions(+), 47 deletions(-) diff --git a/packages/collection/__tests__/collection.test.ts b/packages/collection/__tests__/collection.test.ts index 0ea401fd44e2..ea8f918bdce6 100644 --- a/packages/collection/__tests__/collection.test.ts +++ b/packages/collection/__tests__/collection.test.ts @@ -70,12 +70,20 @@ describe('at() tests', () => { expect(coll.at(0)).toStrictEqual(1); }); + test('positive non-integer index', () => { + expect(coll.at(1.5)).toStrictEqual(2); + }); + test('negative index', () => { expect(coll.at(-1)).toStrictEqual(3); }); + test('negative non-integer index', () => { + expect(coll.at(-2.5)).toStrictEqual(2); + }); + test('invalid positive index', () => { - expect(coll.at(4)).toBeUndefined(); + expect(coll.at(3)).toBeUndefined(); }); test('invalid negative index', () => { @@ -432,12 +440,20 @@ describe('keyAt() tests', () => { expect(coll.keyAt(0)).toStrictEqual('a'); }); + test('positive non-integer index', () => { + expect(coll.keyAt(1.5)).toStrictEqual('b'); + }); + test('negative index', () => { expect(coll.keyAt(-1)).toStrictEqual('c'); }); + test('negative non-integer index', () => { + expect(coll.keyAt(-2.5)).toStrictEqual('b'); + }); + test('invalid positive index', () => { - expect(coll.keyAt(4)).toBeUndefined(); + expect(coll.keyAt(3)).toBeUndefined(); }); test('invalid negative index', () => { diff --git a/packages/collection/src/collection.ts b/packages/collection/src/collection.ts index 19fc01da9b11..5fa11cb1ebb9 100644 --- a/packages/collection/src/collection.ts +++ b/packages/collection/src/collection.ts @@ -85,9 +85,16 @@ export class Collection extends Map { public first(amount?: number): Value | Value[] | undefined { if (amount === undefined) return this.values().next().value; if (amount < 0) return this.last(amount * -1); - amount = Math.min(this.size, amount); + if (amount >= this.size) return [...this.values()]; + const iter = this.values(); - return Array.from({ length: amount }, (): Value => iter.next().value!); + // eslint-disable-next-line unicorn/no-new-array + const results: Value[] = new Array(amount); + for (let index = 0; index < amount; index++) { + results[index] = iter.next().value!; + } + + return results; } /** @@ -102,9 +109,16 @@ export class Collection extends Map { public firstKey(amount?: number): Key | Key[] | undefined { if (amount === undefined) return this.keys().next().value; if (amount < 0) return this.lastKey(amount * -1); - amount = Math.min(this.size, amount); + if (amount >= this.size) return [...this.keys()]; + const iter = this.keys(); - return Array.from({ length: amount }, (): Key => iter.next().value!); + // eslint-disable-next-line unicorn/no-new-array + const results: Key[] = new Array(amount); + for (let index = 0; index < amount; index++) { + results[index] = iter.next().value!; + } + + return results; } /** @@ -117,11 +131,12 @@ export class Collection extends Map { public last(): Value | undefined; public last(amount: number): Value[]; public last(amount?: number): Value | Value[] | undefined { - const arr = [...this.values()]; - if (amount === undefined) return arr[arr.length - 1]; - if (amount < 0) return this.first(amount * -1); + if (amount === undefined) return this.at(-1); if (!amount) return []; - return arr.slice(-amount); + if (amount < 0) return this.first(amount * -1); + + const arr = [...this.values()]; + return arr.slice(amount * -1); } /** @@ -134,11 +149,12 @@ export class Collection extends Map { public lastKey(): Key | undefined; public lastKey(amount: number): Key[]; public lastKey(amount?: number): Key | Key[] | undefined { - const arr = [...this.keys()]; - if (amount === undefined) return arr[arr.length - 1]; - if (amount < 0) return this.firstKey(amount * -1); + if (amount === undefined) return this.keyAt(-1); if (!amount) return []; - return arr.slice(-amount); + if (amount < 0) return this.firstKey(amount * -1); + + const arr = [...this.keys()]; + return arr.slice(amount * -1); } /** @@ -148,10 +164,21 @@ export class Collection extends Map { * * @param index - The index of the element to obtain */ - public at(index: number) { - index = Math.floor(index); - const arr = [...this.values()]; - return arr.at(index); + public at(index: number): Value | undefined { + index = Math.trunc(index); + if (index >= 0) { + if (index >= this.size) return undefined; + } else { + index += this.size; + if (index < 0) return undefined; + } + + const iter = this.values(); + for (let skip = 0; skip < index; skip++) { + iter.next(); + } + + return iter.next().value!; } /** @@ -161,10 +188,21 @@ export class Collection extends Map { * * @param index - The index of the key to obtain */ - public keyAt(index: number) { - index = Math.floor(index); - const arr = [...this.keys()]; - return arr.at(index); + public keyAt(index: number): Key | undefined { + index = Math.trunc(index); + if (index >= 0) { + if (index >= this.size) return undefined; + } else { + index += this.size; + if (index < 0) return undefined; + } + + const iter = this.keys(); + for (let skip = 0; skip < index; skip++) { + iter.next(); + } + + return iter.next().value!; } /** @@ -176,13 +214,17 @@ export class Collection extends Map { public random(): Value | undefined; public random(amount: number): Value[]; public random(amount?: number): Value | Value[] | undefined { - const arr = [...this.values()]; - if (amount === undefined) return arr[Math.floor(Math.random() * arr.length)]; - if (!arr.length || !amount) return []; - return Array.from( - { length: Math.min(amount, arr.length) }, - (): Value => arr.splice(Math.floor(Math.random() * arr.length), 1)[0]!, - ); + if (amount === undefined) return this.at(Math.floor(Math.random() * this.size)); + amount = Math.min(this.size, amount); + if (!amount) return []; + + const values = [...this.values()]; + for (let sourceIndex = 0; sourceIndex < amount; sourceIndex++) { + const targetIndex = sourceIndex + Math.floor(Math.random() * (values.length - sourceIndex)); + [values[sourceIndex], values[targetIndex]] = [values[targetIndex]!, values[sourceIndex]!]; + } + + return values.slice(0, amount); } /** @@ -194,13 +236,17 @@ export class Collection extends Map { public randomKey(): Key | undefined; public randomKey(amount: number): Key[]; public randomKey(amount?: number): Key | Key[] | undefined { - const arr = [...this.keys()]; - if (amount === undefined) return arr[Math.floor(Math.random() * arr.length)]; - if (!arr.length || !amount) return []; - return Array.from( - { length: Math.min(amount, arr.length) }, - (): Key => arr.splice(Math.floor(Math.random() * arr.length), 1)[0]!, - ); + if (amount === undefined) return this.keyAt(Math.floor(Math.random() * this.size)); + amount = Math.min(this.size, amount); + if (!amount) return []; + + const keys = [...this.keys()]; + for (let sourceIndex = 0; sourceIndex < amount; sourceIndex++) { + const targetIndex = sourceIndex + Math.floor(Math.random() * (keys.length - sourceIndex)); + [keys[sourceIndex], keys[targetIndex]] = [keys[targetIndex]!, keys[sourceIndex]!]; + } + + return keys.slice(0, amount); } /** @@ -511,10 +557,14 @@ export class Collection extends Map { if (typeof fn !== 'function') throw new TypeError(`${fn} is not a function`); if (thisArg !== undefined) fn = fn.bind(thisArg); const iter = this.entries(); - return Array.from({ length: this.size }, (): NewValue => { + // eslint-disable-next-line unicorn/no-new-array + const results: NewValue[] = new Array(this.size); + for (let index = 0; index < this.size; index++) { const [key, value] = iter.next().value!; - return fn(value, key, this); - }); + results[index] = fn(value, key, this); + } + + return results; } /** @@ -959,12 +1009,14 @@ export class Collection extends Map { const hasInSelf = this.has(key); const hasInOther = other.has(key); - if (hasInSelf && hasInOther) { - const result = whenInBoth(this.get(key)!, other.get(key)!, key); - if (result.keep) coll.set(key, result.value); - } else if (hasInSelf) { - const result = whenInSelf(this.get(key)!, key); - if (result.keep) coll.set(key, result.value); + if (hasInSelf) { + if (hasInOther) { + const result = whenInBoth(this.get(key)!, other.get(key)!, key); + if (result.keep) coll.set(key, result.value); + } else { + const result = whenInSelf(this.get(key)!, key); + if (result.keep) coll.set(key, result.value); + } } else if (hasInOther) { const result = whenInOther(other.get(key)!, key); if (result.keep) coll.set(key, result.value); @@ -995,8 +1047,8 @@ export class Collection extends Map { * collection.sorted((userA, userB) => userA.createdTimestamp - userB.createdTimestamp); * ``` */ - public toSorted(compareFunction: Comparator = Collection.defaultSort) { - return new this.constructor[Symbol.species](this).sort((av, bv, ak, bk) => compareFunction(av, bv, ak, bk)); + public toSorted(compareFunction: Comparator = Collection.defaultSort): Collection { + return new this.constructor[Symbol.species](this).sort(compareFunction); } public toJSON() { From c97310681da0fab68ac17fb100fbfcb249dec402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= <9092381+Renegade334@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:12:53 +0000 Subject: [PATCH 2/2] types(collection): simplify ambient constructor declaration (#10549) - deduplicates constructor definition - removes Collection's "internal" JSDoc description block - removes unnecessary `extends` clause Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/collection/src/collection.ts | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/collection/src/collection.ts b/packages/collection/src/collection.ts index 5fa11cb1ebb9..8f842ef68ad6 100644 --- a/packages/collection/src/collection.ts +++ b/packages/collection/src/collection.ts @@ -1,14 +1,4 @@ /* eslint-disable no-param-reassign */ -/** - * @internal - */ -export interface CollectionConstructor { - new (): Collection; - new (entries?: readonly (readonly [Key, Value])[] | null): Collection; - new (iterable: Iterable): Collection; - readonly prototype: Collection; - readonly [Symbol.species]: CollectionConstructor; -} /** * Represents an immutable version of a collection @@ -19,13 +9,13 @@ export type ReadonlyCollection = Omit< > & ReadonlyMap; -/** - * Separate interface for the constructor so that emitted js does not have a constructor that overwrites itself - * - * @internal - */ -export interface Collection extends Map { - constructor: CollectionConstructor; +export interface Collection { + /** + * Ambient declaration to allow `this.constructor[@@species]` in class methods. + * + * @internal + */ + constructor: typeof Collection & { readonly [Symbol.species]: typeof Collection }; } /**