From 17835fb59db026a417c0ada86431ded331fa9c43 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 18 Apr 2024 16:59:48 -0400 Subject: [PATCH 1/7] Ensure we have a mostly-sorted items array to speed up sorting --- .../src/entities/sorted_state_adapter.ts | 45 +++++++++- .../tests/sorted_state_adapter.test.ts | 89 ++++++++++++++++++- 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index dc7301451..5fbd89911 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -76,6 +76,7 @@ export function createSortedStateAdapter( state: R, ): void { let appliedUpdates = false + let replacedIds = false for (let update of updates) { const entity: T | undefined = (state.entities as Record)[update.id] @@ -88,13 +89,16 @@ export function createSortedStateAdapter( Object.assign(entity, update.changes) const newId = selectId(entity) if (update.id !== newId) { + replacedIds = true delete (state.entities as Record)[update.id] + const oldIndex = (state.ids as Id[]).indexOf(update.id) + state.ids[oldIndex] = newId ;(state.entities as Record)[newId] = entity } } if (appliedUpdates) { - resortEntities(state) + resortEntities(state, [], replacedIds) } } @@ -136,11 +140,44 @@ export function createSortedStateAdapter( ;(state.entities as Record)[selectId(model)] = model }) - resortEntities(state) + resortEntities(state, models) } - function resortEntities(state: R) { - const allEntities = Object.values(state.entities) as T[] + function resortEntities( + state: R, + addedItems: readonly T[] = [], + replacedIds = false, + ) { + let allEntities: T[] + + allEntities = Object.values(state.entities) as T[] + if (replacedIds) { + // This is a really annoying edge case. Just figure this out from scratch + // rather than try to be clever. This will be more expensive since it isn't sorted right. + allEntities = Object.values(state.entities) as T[] + } else { + // We're starting with an already-sorted list. + let existingIds = state.ids + + if (addedItems.length) { + // There's a couple edge cases where we can have duplicate item IDs. + // Ensure we don't have duplicates. + const uniqueIds = new Set(existingIds as Id[]) + + addedItems.forEach((item) => { + uniqueIds.add(selectId(item)) + }) + existingIds = Array.from(uniqueIds) + } + + // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. + // Make this a sorta-mostly-sorted array. + allEntities = existingIds.map( + (id) => (state.entities as Record)[id as Id], + ) + } + + // Now when we sort, things should be _close_ already, and fewer comparisons are needed. allEntities.sort(sort) const newSortedIds = allEntities.map(selectId) diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index 9683715fb..8fb75c392 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -1,6 +1,11 @@ import type { EntityAdapter, EntityState } from '../models' import { createEntityAdapter } from '../create_adapter' -import { createAction, createSlice, configureStore } from '@reduxjs/toolkit' +import { + createAction, + createSlice, + configureStore, + nanoid, +} from '@reduxjs/toolkit' import type { BookModel } from './fixtures/book' import { TheGreatGatsby, @@ -247,7 +252,7 @@ describe('Sorted State Adapter', () => { const { ids, entities } = withUpdated - expect(ids.length).toBe(2) + expect(ids).toEqual(['a', 'c']) expect(entities.a).toBeTruthy() expect(entities.b).not.toBeTruthy() expect(entities.c).toBeTruthy() @@ -584,6 +589,86 @@ describe('Sorted State Adapter', () => { expect(withUpdate.entities['b']!.title).toBe(book1.title) }) + it('should minimize the amount of sorting work needed', () => { + const PARAMETERS = { + NUM_ITEMS: 10_000, + } + + type Entity = { id: string; name: string; position: number } + + let numSorts = 0 + + const adaptor = createEntityAdapter({ + selectId: (entity: Entity) => entity.id, + sortComparer: (a, b) => { + numSorts++ + if (a.position < b.position) return -1 + else if (a.position > b.position) return 1 + return 0 + }, + }) + + const initialState: Entity[] = new Array(PARAMETERS.NUM_ITEMS) + .fill(undefined) + .map((x, i) => ({ + name: `${i}`, + position: Math.random(), + id: nanoid(), + })) + + const entitySlice = createSlice({ + name: 'entity', + initialState: adaptor.getInitialState(undefined, initialState), + reducers: { + updateOne: adaptor.updateOne, + upsertOne: adaptor.upsertOne, + upsertMany: adaptor.upsertMany, + }, + }) + + const store = configureStore({ + reducer: { + entity: entitySlice.reducer, + }, + middleware: (getDefaultMiddleware) => { + return getDefaultMiddleware({ + serializableCheck: false, + immutableCheck: false, + }) + }, + }) + + store.dispatch( + entitySlice.actions.upsertOne({ + id: nanoid(), + position: Math.random(), + name: 'test', + }), + ) + + // These numbers will vary because of the randomness, but generally + // with 10K items the old code had 200K+ sort calls, while the new code + // is around 130K sort calls. + expect(numSorts).toBeLessThan(200_000) + + const { ids } = store.getState().entity + const middleItemId = ids[(ids.length / 2) | 0] + + numSorts = 0 + + store.dispatch( + // Move this middle item near the end + entitySlice.actions.updateOne({ + id: middleItemId, + changes: { + position: 0.99999, + }, + }), + ) + // The old code was around 120K, the new code is around 10K. + expect(numSorts).toBeLessThan(25_000) + }) + describe('can be used mutably when wrapped in createNextState', () => { test('removeAll', () => { const withTwo = adapter.addMany(state, [TheGreatGatsby, AnimalFarm]) From 6c45a99c84a3a2fea99e8e68e6fb11d854e02340 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 20 Apr 2024 18:12:50 -0400 Subject: [PATCH 2/7] Insertion passes but slowly --- .../src/entities/sorted_state_adapter.ts | 360 +++++++++++++++++- .../tests/sorted_state_adapter.test.ts | 150 ++++++-- 2 files changed, 472 insertions(+), 38 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index 5fbd89911..b00cd6900 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -1,3 +1,4 @@ +import { current, isDraft } from 'immer' import type { IdSelector, Comparer, @@ -14,6 +15,40 @@ import { splitAddedUpdatedEntities, } from './utils' +export function findInsertIndex( + sortedItems: T[], + item: T, + comparisonFunction: Comparer, +): number { + let lowIndex = 0 + let highIndex = sortedItems.length + while (lowIndex < highIndex) { + let middleIndex = (lowIndex + highIndex) >>> 1 + const currentItem = sortedItems[middleIndex] + const res = comparisonFunction(item, currentItem) + // console.log('Res: ', item, currentItem, res) + if (res >= 0) { + lowIndex = middleIndex + 1 + } else { + highIndex = middleIndex + } + } + + return lowIndex +} + +export function insert( + sortedItems: T[], + item: T, + comparisonFunction: Comparer, +): T[] { + const insertAtIndex = findInsertIndex(sortedItems, item, comparisonFunction) + + sortedItems.splice(insertAtIndex, 0, item) + + return sortedItems +} + export function createSortedStateAdapter( selectId: IdSelector, sort: Comparer, @@ -38,7 +73,7 @@ export function createSortedStateAdapter( ) if (models.length !== 0) { - merge(models, state) + mergeFunction(state, models) } } @@ -52,7 +87,11 @@ export function createSortedStateAdapter( ): void { newEntities = ensureEntitiesArray(newEntities) if (newEntities.length !== 0) { - merge(newEntities, state) + //const updatedIds = new Set(newEntities.map((item) => selectId(item))) + for (const item of newEntities) { + delete (state.entities as Record)[selectId(item)] + } + mergeFunction(state, newEntities) } } @@ -77,6 +116,7 @@ export function createSortedStateAdapter( ): void { let appliedUpdates = false let replacedIds = false + const updatedIds = new Set() for (let update of updates) { const entity: T | undefined = (state.entities as Record)[update.id] @@ -88,9 +128,11 @@ export function createSortedStateAdapter( Object.assign(entity, update.changes) const newId = selectId(entity) + updatedIds.add(newId) if (update.id !== newId) { replacedIds = true delete (state.entities as Record)[update.id] + updatedIds.delete(update.id) const oldIndex = (state.ids as Id[]).indexOf(update.id) state.ids[oldIndex] = newId ;(state.entities as Record)[newId] = entity @@ -98,7 +140,8 @@ export function createSortedStateAdapter( } if (appliedUpdates) { - resortEntities(state, [], replacedIds) + mergeFunction(state, [], updatedIds, replacedIds) + // resortEntities(state, [], replacedIds) } } @@ -143,6 +186,317 @@ export function createSortedStateAdapter( resortEntities(state, models) } + function cleanItem(item: T) { + return isDraft(item) ? current(item) : item + } + + type MergeFunction = ( + state: R, + addedItems: readonly T[], + updatedIds?: Set, + replacedIds?: boolean, + ) => void + + // const mergeFunction: MergeFunction = (state, addedItems) => { + // const actualMergeFunction: MergeFunction = mergeOriginal + // console.log('Merge function: ', actualMergeFunction.name) + // actualMergeFunction(state, addedItems) + // } + + const mergeOriginal: MergeFunction = (state, addedItems) => { + // Insert/overwrite all new/updated + addedItems.forEach((model) => { + ;(state.entities as Record)[selectId(model)] = model + }) + resortEntities(state) + + function resortEntities(state: R) { + const allEntities = Object.values(state.entities) as T[] + allEntities.sort(sort) + const newSortedIds = allEntities.map(selectId) + const { ids } = state + if (!areArraysEqual(ids, newSortedIds)) { + state.ids = newSortedIds + } + } + } + + const mergeLenz: MergeFunction = ( + state, + addedItems: readonly T[], + updatedIds, + replacedIds, + ) => { + const entities = state.entities as Record + let ids = state.ids as Id[] + if (replacedIds) { + ids = Array.from(new Set(ids)) + } + const oldEntities = ids // Array.from(new Set(state.ids as Id[])) + .map((id) => entities[id]) + .filter(Boolean) + + let newSortedIds: Id[] = [] + const seenIds = new Set() + + if (addedItems.length) { + const newEntities = addedItems.slice().sort(sort) + + // Insert/overwrite all new/updated + newEntities.forEach((model) => { + entities[selectId(model)] = model + }) + + let o = 0, + n = 0 + while (o < oldEntities.length && n < newEntities.length) { + const oldEntity = oldEntities[o] as T, + oldId = selectId(oldEntity), + newEntity = newEntities[n], + newId = selectId(newEntity) + + if (seenIds.has(newId)) { + n++ + continue + } + + const comparison = sort(oldEntity, newEntity) + if (comparison < 0) { + // Sort the existing item first + newSortedIds.push(oldId) + seenIds.add(oldId) + o++ + continue + } + + if (comparison > 0) { + // Sort the new item first + newSortedIds.push(newId) + seenIds.add(newId) + n++ + continue + } + // The items are equivalent. Maintain stable sorting by + // putting the existing item first. + newSortedIds.push(oldId) + seenIds.add(oldId) + o++ + newSortedIds.push(newId) + seenIds.add(newId) + n++ + } + // Add any remaining existing items + while (o < oldEntities.length) { + newSortedIds.push(selectId(oldEntities[o])) + o++ + } + // Add any remaining new items + while (n < newEntities.length) { + newSortedIds.push(selectId(newEntities[n])) + n++ + } + } else if (updatedIds) { + oldEntities.sort(sort) + newSortedIds = oldEntities.map(selectId) + } + + if (!areArraysEqual(state.ids, newSortedIds)) { + state.ids = newSortedIds + } + } + + const mergeInsertion: MergeFunction = ( + state, + addedItems, + updatedIds, + replacedIds, + ) => { + const entities = state.entities as Record + let ids = state.ids as Id[] + if (replacedIds) { + ids = Array.from(new Set(ids)) + } + + // //let sortedEntities: T[] = [] + + // const wasPreviouslyEmpty = ids.length === 0 + + // let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) + // .map((id) => entities[id]) + // .filter(Boolean) + + // if (addedItems.length) { + // if (wasPreviouslyEmpty) { + // sortedEntities = addedItems.slice().sort() + // } + + // for (const item of addedItems) { + // entities[selectId(item)] = item + // if (!wasPreviouslyEmpty) { + // insert(sortedEntities, item, sort) + // } + // } + // } + const sortedEntities = ids // Array.from(new Set(state.ids as Id[])) + .map((id) => entities[id]) + .filter(Boolean) + + // let oldIds = state.ids as Id[] + // // if (updatedIds) { + // // oldIds = oldIds.filter((id) => !updatedIds.has(id)) + // // const updatedItems = Array.from(updatedIds) + // // .map((id) => entities[id]) + // // .filter(Boolean) + // // models = updatedItems.concat(models) + // // } + // // console.log('Old IDs: ', oldIds) + // const sortedEntities = oldIds + // .map((id) => (state.entities as Record)[id as Id]) + // .filter(Boolean) + + // Insert/overwrite all new/updated + addedItems.forEach((item) => { + entities[selectId(item)] = item + // console.log('Inserting: ', isDraft(item) ? current(item) : item) + insert(sortedEntities, item, sort) + }) + + if (updatedIds?.size) { + sortedEntities.sort(sort) + } + + const newSortedIds = sortedEntities.map(selectId) + // console.log('New sorted IDs: ', newSortedIds) + if (!areArraysEqual(state.ids, newSortedIds)) { + state.ids = newSortedIds + } + } + + const mergeInitialPR: MergeFunction = ( + state, + addedItems, + updatedIds, + replacedIds, + ) => { + // Insert/overwrite all new/updated + addedItems.forEach((model) => { + ;(state.entities as Record)[selectId(model)] = model + }) + + let allEntities: T[] + + if (replacedIds) { + // This is a really annoying edge case. Just figure this out from scratch + // rather than try to be clever. This will be more expensive since it isn't sorted right. + allEntities = Object.values(state.entities) as T[] + } else { + // We're starting with an already-sorted list. + let existingIds = state.ids + + if (addedItems.length) { + // There's a couple edge cases where we can have duplicate item IDs. + // Ensure we don't have duplicates. + const uniqueIds = new Set(existingIds as Id[]) + + addedItems.forEach((item) => { + uniqueIds.add(selectId(item)) + }) + existingIds = Array.from(uniqueIds) + } + + // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. + // Make this a sorta-mostly-sorted array. + allEntities = existingIds.map( + (id) => (state.entities as Record)[id as Id], + ) + } + + // Now when we sort, things should be _close_ already, and fewer comparisons are needed. + allEntities.sort(sort) + + const newSortedIds = allEntities.map(selectId) + const { ids } = state + + if (!areArraysEqual(ids, newSortedIds)) { + state.ids = newSortedIds + } + } + + const mergeTweakedPR: MergeFunction = ( + state, + addedItems, + updatedIds, + replacedIds, + ) => { + let allEntities: T[] + + let existingIds = state.ids + const uniqueIds = new Set(existingIds as Id[]) + existingIds = Array.from(uniqueIds) + + if (addedItems.length) { + // There's a couple edge cases where we can have duplicate item IDs. + // Ensure we don't have duplicates. + + addedItems.forEach((item) => { + ;(state.entities as Record)[selectId(item)] = item + uniqueIds.add(selectId(item)) + }) + existingIds = Array.from(uniqueIds) + } + + // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. + // Make this a sorta-mostly-sorted array. + allEntities = existingIds.map( + (id) => (state.entities as Record)[id as Id], + ) + + // if (replacedIds) { + // // There's a couple edge cases where we can have duplicate item IDs. + // // Ensure we don't have duplicates. + // const uniqueIds = new Set(existingIds as Id[]) + // // This is a really annoying edge case. Just figure this out from scratch + // // rather than try to be clever. This will be more expensive since it isn't sorted right. + // allEntities = state.ids.map( + // (id) => (state.entities as Record)[id as Id], + // ) + // } else { + // // We're starting with an already-sorted list. + // let existingIds = state.ids + + // if (addedItems.length) { + // // There's a couple edge cases where we can have duplicate item IDs. + // // Ensure we don't have duplicates. + // const uniqueIds = new Set(existingIds as Id[]) + + // addedItems.forEach((item) => { + + // ;(state.entities as Record)[selectId(item)] = item + // uniqueIds.add(selectId(item)) + // }) + // existingIds = Array.from(uniqueIds) + // } + + // // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. + // // Make this a sorta-mostly-sorted array. + // allEntities = existingIds.map( + // (id) => (state.entities as Record)[id as Id], + // ) + // } + + // Now when we sort, things should be _close_ already, and fewer comparisons are needed. + allEntities.sort(sort) + + const newSortedIds = allEntities.map(selectId) + const { ids } = state + + if (!areArraysEqual(ids, newSortedIds)) { + state.ids = newSortedIds + } + } + + const mergeFunction: MergeFunction = mergeInsertion + function resortEntities( state: R, addedItems: readonly T[] = [], diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index 8fb75c392..b1141c2c9 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -237,11 +237,11 @@ describe('Sorted State Adapter', () => { }) it('Replaces an existing entity if you change the ID while updating', () => { - const withAdded = adapter.setAll(state, [ - { id: 'a', title: 'First' }, - { id: 'b', title: 'Second' }, - { id: 'c', title: 'Third' }, - ]) + const a = { id: 'a', title: 'First' } + const b = { id: 'b', title: 'Second' } + const c = { id: 'c', title: 'Third' } + const d = { id: 'd', title: 'Fourth' } + const withAdded = adapter.setAll(state, [a, b, c]) const withUpdated = adapter.updateOne(withAdded, { id: 'b', @@ -368,6 +368,8 @@ describe('Sorted State Adapter', () => { ], ) + expect(withInitialItems.ids).toEqual(['A', 'B', 'C', 'D', 'E']) + const updated = sortedItemsAdapter.updateOne(withInitialItems, { id: 'C', changes: { ts: 5 }, @@ -590,9 +592,8 @@ describe('Sorted State Adapter', () => { }) it('should minimize the amount of sorting work needed', () => { - const PARAMETERS = { - NUM_ITEMS: 10_000, - } + const INITIAL_ITEMS = 100_000 + const ADDED_ITEMS = 1000 type Entity = { id: string; name: string; position: number } @@ -608,21 +609,25 @@ describe('Sorted State Adapter', () => { }, }) - const initialState: Entity[] = new Array(PARAMETERS.NUM_ITEMS) - .fill(undefined) - .map((x, i) => ({ - name: `${i}`, - position: Math.random(), - id: nanoid(), - })) + function generateItems(count: number) { + const items: readonly Entity[] = new Array(count) + .fill(undefined) + .map((x, i) => ({ + name: `${i}`, + position: Math.random(), + id: nanoid(), + })) + return items + } const entitySlice = createSlice({ name: 'entity', - initialState: adaptor.getInitialState(undefined, initialState), + initialState: adaptor.getInitialState(), reducers: { updateOne: adaptor.updateOne, upsertOne: adaptor.upsertOne, upsertMany: adaptor.upsertMany, + addMany: adaptor.addMany, }, }) @@ -638,35 +643,110 @@ describe('Sorted State Adapter', () => { }, }) - store.dispatch( - entitySlice.actions.upsertOne({ - id: nanoid(), - position: Math.random(), - name: 'test', - }), - ) + numSorts = 0 + + function measureComparisons(name: string, cb: () => void) { + numSorts = 0 + const start = new Date().getTime() + cb() + const end = new Date().getTime() + const duration = end - start + + console.log( + `${name}: sortComparer called ${numSorts.toLocaleString()} times in ${duration.toLocaleString()}ms`, + numSorts.toLocaleString(), + 'times', + ) + } + + const initialItems = generateItems(INITIAL_ITEMS) + + measureComparisons('Original Setup', () => { + store.dispatch(entitySlice.actions.upsertMany(initialItems)) + }) + + measureComparisons('Insert One (random)', () => { + store.dispatch( + entitySlice.actions.upsertOne({ + id: nanoid(), + position: Math.random(), + name: 'test', + }), + ) + }) + + measureComparisons('Insert One (middle)', () => { + store.dispatch( + entitySlice.actions.upsertOne({ + id: nanoid(), + position: 0.5, + name: 'test', + }), + ) + }) + + measureComparisons('Insert One (end)', () => { + store.dispatch( + entitySlice.actions.upsertOne({ + id: nanoid(), + position: 0.9998, + name: 'test', + }), + ) + }) + + const addedItems = generateItems(ADDED_ITEMS) + measureComparisons('Add Many', () => { + store.dispatch(entitySlice.actions.addMany(addedItems)) + }) // These numbers will vary because of the randomness, but generally // with 10K items the old code had 200K+ sort calls, while the new code // is around 130K sort calls. - expect(numSorts).toBeLessThan(200_000) + // expect(numSorts).toBeLessThan(200_000) const { ids } = store.getState().entity const middleItemId = ids[(ids.length / 2) | 0] - numSorts = 0 + measureComparisons('Update One (end)', () => { + store.dispatch( + // Move this middle item near the end + entitySlice.actions.updateOne({ + id: middleItemId, + changes: { + position: 0.99999, + }, + }), + ) + }) + + measureComparisons('Update One (middle)', () => { + store.dispatch( + // Move this middle item near the end + entitySlice.actions.updateOne({ + id: middleItemId, + changes: { + position: 0.42, + }, + }), + ) + }) + + measureComparisons('Update One (replace)', () => { + store.dispatch( + // Move this middle item near the end + entitySlice.actions.updateOne({ + id: middleItemId, + changes: { + id: nanoid(), + position: 0.98, + }, + }), + ) + }) - store.dispatch( - // Move this middle item near the end - entitySlice.actions.updateOne({ - id: middleItemId, - changes: { - position: 0.99999, - }, - }), - ) // The old code was around 120K, the new code is around 10K. - expect(numSorts).toBeLessThan(25_000) + // expect(numSorts).toBeLessThan(25_000) }) describe('can be used mutably when wrapped in createNextState', () => { From 8db3f7fca8d16892f367e47253a3111585de30ad Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 20 Apr 2024 18:33:54 -0400 Subject: [PATCH 3/7] Insertion passing faster, rename "comparer" --- .../src/entities/sorted_state_adapter.ts | 36 ++++++++++--------- .../tests/sorted_state_adapter.test.ts | 2 -- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index b00cd6900..f40c887a2 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -51,7 +51,7 @@ export function insert( export function createSortedStateAdapter( selectId: IdSelector, - sort: Comparer, + comparer: Comparer, ): EntityStateAdapter { type R = DraftableEntityState @@ -212,7 +212,7 @@ export function createSortedStateAdapter( function resortEntities(state: R) { const allEntities = Object.values(state.entities) as T[] - allEntities.sort(sort) + allEntities.sort(comparer) const newSortedIds = allEntities.map(selectId) const { ids } = state if (!areArraysEqual(ids, newSortedIds)) { @@ -240,7 +240,7 @@ export function createSortedStateAdapter( const seenIds = new Set() if (addedItems.length) { - const newEntities = addedItems.slice().sort(sort) + const newEntities = addedItems.slice().sort(comparer) // Insert/overwrite all new/updated newEntities.forEach((model) => { @@ -260,7 +260,7 @@ export function createSortedStateAdapter( continue } - const comparison = sort(oldEntity, newEntity) + const comparison = comparer(oldEntity, newEntity) if (comparison < 0) { // Sort the existing item first newSortedIds.push(oldId) @@ -296,7 +296,7 @@ export function createSortedStateAdapter( n++ } } else if (updatedIds) { - oldEntities.sort(sort) + oldEntities.sort(comparer) newSortedIds = oldEntities.map(selectId) } @@ -319,8 +319,6 @@ export function createSortedStateAdapter( // //let sortedEntities: T[] = [] - // const wasPreviouslyEmpty = ids.length === 0 - // let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) // .map((id) => entities[id]) // .filter(Boolean) @@ -337,10 +335,12 @@ export function createSortedStateAdapter( // } // } // } - const sortedEntities = ids // Array.from(new Set(state.ids as Id[])) + let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) .map((id) => entities[id]) .filter(Boolean) + const wasPreviouslyEmpty = sortedEntities.length === 0 + // let oldIds = state.ids as Id[] // // if (updatedIds) { // // oldIds = oldIds.filter((id) => !updatedIds.has(id)) @@ -355,14 +355,18 @@ export function createSortedStateAdapter( // .filter(Boolean) // Insert/overwrite all new/updated - addedItems.forEach((item) => { + for (const item of addedItems) { entities[selectId(item)] = item // console.log('Inserting: ', isDraft(item) ? current(item) : item) - insert(sortedEntities, item, sort) - }) + if (!wasPreviouslyEmpty) { + insert(sortedEntities, item, comparer) + } + } - if (updatedIds?.size) { - sortedEntities.sort(sort) + if (wasPreviouslyEmpty) { + sortedEntities = addedItems.slice().sort(comparer) + } else if (updatedIds?.size) { + sortedEntities.sort(comparer) } const newSortedIds = sortedEntities.map(selectId) @@ -412,7 +416,7 @@ export function createSortedStateAdapter( } // Now when we sort, things should be _close_ already, and fewer comparisons are needed. - allEntities.sort(sort) + allEntities.sort(comparer) const newSortedIds = allEntities.map(selectId) const { ids } = state @@ -485,7 +489,7 @@ export function createSortedStateAdapter( // } // Now when we sort, things should be _close_ already, and fewer comparisons are needed. - allEntities.sort(sort) + allEntities.sort(comparer) const newSortedIds = allEntities.map(selectId) const { ids } = state @@ -532,7 +536,7 @@ export function createSortedStateAdapter( } // Now when we sort, things should be _close_ already, and fewer comparisons are needed. - allEntities.sort(sort) + allEntities.sort(comparer) const newSortedIds = allEntities.map(selectId) const { ids } = state diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index b1141c2c9..cfdded6a0 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -654,8 +654,6 @@ describe('Sorted State Adapter', () => { console.log( `${name}: sortComparer called ${numSorts.toLocaleString()} times in ${duration.toLocaleString()}ms`, - numSorts.toLocaleString(), - 'times', ) } From bece99bebd8d2f18e32995a5cc55726d050f576e Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 20 Apr 2024 19:26:08 -0400 Subject: [PATCH 4/7] Add some assertions for perf expectations --- .../src/entities/sorted_state_adapter.ts | 97 +++++++++++++++++++ .../tests/sorted_state_adapter.test.ts | 28 +++++- 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index f40c887a2..aef9ff20b 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -499,6 +499,103 @@ export function createSortedStateAdapter( } } + const mergeJackman: MergeFunction = ( + state, + addedItems, + updatedIds, + replacedIds, + ) => { + const entities = state.entities as Record + let ids = state.ids as Id[] + if (replacedIds) { + ids = Array.from(new Set(ids)) + } + const existingSortedItems = ids // Array.from(new Set(state.ids as Id[])) + .map((id) => entities[id]) + .filter(Boolean) + + function findInsertIndex2( + sortedItems: T[], + item: T, + comparisonFunction: Comparer, + lowIndexOverride?: number, + ): number { + let lowIndex = lowIndexOverride ?? 0 + let highIndex = sortedItems.length + while (lowIndex < highIndex) { + const middleIndex = (lowIndex + highIndex) >>> 1 + const currentItem = sortedItems[middleIndex] + if (comparisonFunction(item, currentItem) > 0) { + lowIndex = middleIndex + 1 + } else { + highIndex = middleIndex + } + } + + return lowIndex + } + + if (addedItems.length) { + const newEntities = addedItems.slice().sort(comparer) + + // Insert/overwrite all new/updated + newEntities.forEach((model) => { + entities[selectId(model)] = model + }) + + const firstInstanceId = newEntities[0] + const lastInstanceId = newEntities[newEntities.length - 1] + + const startIndex = findInsertIndex2( + existingSortedItems, + firstInstanceId, + comparer, + ) + const endIndex = findInsertIndex2( + existingSortedItems, + lastInstanceId, + comparer, + startIndex, + ) + + const overlappingExistingIds = existingSortedItems.slice( + startIndex, + endIndex, + ) + let newIdIndexOfLastInsert = 0 + let lastRelativeInsertIndex = 0 + for (let i = 1; i < newEntities.length; i++) { + const relativeInsertIndex = findInsertIndex2( + overlappingExistingIds, + newEntities[i], + comparer, + lastRelativeInsertIndex, + ) + if (lastRelativeInsertIndex !== relativeInsertIndex) { + const insertIndex = + startIndex + newIdIndexOfLastInsert + lastRelativeInsertIndex + const arrayToInsert = newEntities.slice(newIdIndexOfLastInsert, i) + existingSortedItems.splice(insertIndex, 0, ...arrayToInsert) + newIdIndexOfLastInsert = i + lastRelativeInsertIndex = relativeInsertIndex + } + } + existingSortedItems.splice( + startIndex + newIdIndexOfLastInsert + lastRelativeInsertIndex, + 0, + ...newEntities.slice(newIdIndexOfLastInsert), + ) + } else if (updatedIds?.size) { + existingSortedItems.sort(comparer) + } + + const newSortedIds = existingSortedItems.map(selectId) + + if (!areArraysEqual(ids, newSortedIds)) { + state.ids = newSortedIds + } + } + const mergeFunction: MergeFunction = mergeInsertion function resortEntities( diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index cfdded6a0..f1b34c051 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -592,8 +592,8 @@ describe('Sorted State Adapter', () => { }) it('should minimize the amount of sorting work needed', () => { - const INITIAL_ITEMS = 100_000 - const ADDED_ITEMS = 1000 + const INITIAL_ITEMS = 10_000 + const ADDED_ITEMS = 1_000 type Entity = { id: string; name: string; position: number } @@ -663,6 +663,8 @@ describe('Sorted State Adapter', () => { store.dispatch(entitySlice.actions.upsertMany(initialItems)) }) + expect(numSorts).toBeLessThan(INITIAL_ITEMS * 20) + measureComparisons('Insert One (random)', () => { store.dispatch( entitySlice.actions.upsertOne({ @@ -673,6 +675,8 @@ describe('Sorted State Adapter', () => { ) }) + expect(numSorts).toBeLessThan(50) + measureComparisons('Insert One (middle)', () => { store.dispatch( entitySlice.actions.upsertOne({ @@ -683,6 +687,8 @@ describe('Sorted State Adapter', () => { ) }) + expect(numSorts).toBeLessThan(50) + measureComparisons('Insert One (end)', () => { store.dispatch( entitySlice.actions.upsertOne({ @@ -693,11 +699,15 @@ describe('Sorted State Adapter', () => { ) }) + expect(numSorts).toBeLessThan(50) + const addedItems = generateItems(ADDED_ITEMS) measureComparisons('Add Many', () => { store.dispatch(entitySlice.actions.addMany(addedItems)) }) + expect(numSorts).toBeLessThan(ADDED_ITEMS * 20) + // These numbers will vary because of the randomness, but generally // with 10K items the old code had 200K+ sort calls, while the new code // is around 130K sort calls. @@ -718,6 +728,12 @@ describe('Sorted State Adapter', () => { ) }) + const SORTING_COUNT_BUFFER = 100 + + expect(numSorts).toBeLessThan( + INITIAL_ITEMS + ADDED_ITEMS + SORTING_COUNT_BUFFER, + ) + measureComparisons('Update One (middle)', () => { store.dispatch( // Move this middle item near the end @@ -730,6 +746,10 @@ describe('Sorted State Adapter', () => { ) }) + expect(numSorts).toBeLessThan( + INITIAL_ITEMS + ADDED_ITEMS + SORTING_COUNT_BUFFER, + ) + measureComparisons('Update One (replace)', () => { store.dispatch( // Move this middle item near the end @@ -743,6 +763,10 @@ describe('Sorted State Adapter', () => { ) }) + expect(numSorts).toBeLessThan( + INITIAL_ITEMS + ADDED_ITEMS + SORTING_COUNT_BUFFER, + ) + // The old code was around 120K, the new code is around 10K. // expect(numSorts).toBeLessThan(25_000) }) From d60cf844e3fedc87561fbdddc2013a18eaef99fa Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Tue, 23 Apr 2024 00:09:12 +0200 Subject: [PATCH 5/7] Try speeding up entity adapter by minimizing Immer reads --- .../src/entities/sorted_state_adapter.ts | 54 ++++++++++++++----- packages/toolkit/src/entities/utils.ts | 15 ++++-- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index aef9ff20b..e70bca548 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -13,6 +13,7 @@ import { selectIdValue, ensureEntitiesArray, splitAddedUpdatedEntities, + getCurrent, } from './utils' export function findInsertIndex( @@ -65,11 +66,17 @@ export function createSortedStateAdapter( function addManyMutably( newEntities: readonly T[] | Record, state: R, + existingIds?: Id[], ): void { newEntities = ensureEntitiesArray(newEntities) + const existingKeys = new Set( + existingIds ?? (current(state.ids) as Id[]), + ) + const models = newEntities.filter( - (model) => !(selectIdValue(model, selectId) in state.entities), + // (model) => !(selectId(model) in state.entities), + (model) => !existingKeys.has(selectIdValue(model, selectId)), ) if (models.length !== 0) { @@ -103,7 +110,7 @@ export function createSortedStateAdapter( state.entities = {} as Record state.ids = [] - addManyMutably(newEntities, state) + addManyMutably(newEntities, state, []) } function updateOneMutably(update: Update, state: R): void { @@ -153,14 +160,18 @@ export function createSortedStateAdapter( newEntities: readonly T[] | Record, state: R, ): void { - const [added, updated] = splitAddedUpdatedEntities( + const [added, updated, existingIdsArray] = splitAddedUpdatedEntities( newEntities, selectId, state, ) - updateManyMutably(updated, state) - addManyMutably(added, state) + if (updated.length) { + updateManyMutably(updated, state) + } + if (added.length) { + addManyMutably(added, state, existingIdsArray) + } } function areArraysEqual(a: readonly unknown[], b: readonly unknown[]) { @@ -311,10 +322,14 @@ export function createSortedStateAdapter( updatedIds, replacedIds, ) => { - const entities = state.entities as Record - let ids = state.ids as Id[] + const currentEntities = getCurrent(state.entities) as Record + const currentIds = getCurrent(state.ids) as Id[] + // const entities = state.entities as Record + const stateEntities = state.entities as Record + // let ids = state.ids as Id[] + let ids = currentIds if (replacedIds) { - ids = Array.from(new Set(ids)) + ids = Array.from(new Set(currentIds)) } // //let sortedEntities: T[] = [] @@ -335,9 +350,16 @@ export function createSortedStateAdapter( // } // } // } - let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) - .map((id) => entities[id]) - .filter(Boolean) + let sortedEntities: T[] = [] + for (const id of ids) { + const entity = currentEntities[id] + if (entity) { + sortedEntities.push(entity) + } + } + // let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) + // .map((id) => currentEntities[id]) + // .filter(Boolean) const wasPreviouslyEmpty = sortedEntities.length === 0 @@ -356,7 +378,7 @@ export function createSortedStateAdapter( // Insert/overwrite all new/updated for (const item of addedItems) { - entities[selectId(item)] = item + stateEntities[selectId(item)] = item // console.log('Inserting: ', isDraft(item) ? current(item) : item) if (!wasPreviouslyEmpty) { insert(sortedEntities, item, comparer) @@ -366,12 +388,18 @@ export function createSortedStateAdapter( if (wasPreviouslyEmpty) { sortedEntities = addedItems.slice().sort(comparer) } else if (updatedIds?.size) { + for (const updatedId of updatedIds) { + const item: T = currentEntities[updatedId] + // const currentIndex = sortedEntities.indexOf(item) + // const newIndex = findInsertIndex(sortedEntities, item, comparer) + // console.log('Item: ', updatedId, { currentIndex, newIndex }) + } sortedEntities.sort(comparer) } const newSortedIds = sortedEntities.map(selectId) // console.log('New sorted IDs: ', newSortedIds) - if (!areArraysEqual(state.ids, newSortedIds)) { + if (!areArraysEqual(currentIds, newSortedIds)) { state.ids = newSortedIds } } diff --git a/packages/toolkit/src/entities/utils.ts b/packages/toolkit/src/entities/utils.ts index 5e3fb92fc..6ed06d518 100644 --- a/packages/toolkit/src/entities/utils.ts +++ b/packages/toolkit/src/entities/utils.ts @@ -1,3 +1,4 @@ +import { current, isDraft } from 'immer' import type { IdSelector, Update, @@ -35,23 +36,31 @@ export function ensureEntitiesArray( return entities } +export function getCurrent(value: T): T { + return isDraft(value) ? current(value) : value +} + export function splitAddedUpdatedEntities( newEntities: readonly T[] | Record, selectId: IdSelector, state: DraftableEntityState, -): [T[], Update[]] { +): [T[], Update[], Id[]] { newEntities = ensureEntitiesArray(newEntities) + const existingIdsArray = current(state.ids) as Id[] + const existingIds = new Set(existingIdsArray) + const added: T[] = [] const updated: Update[] = [] for (const entity of newEntities) { const id = selectIdValue(entity, selectId) - if (id in state.entities) { + if (existingIds.has(id)) { + // if (id in state.entities) { updated.push({ id, changes: entity }) } else { added.push(entity) } } - return [added, updated] + return [added, updated, existingIdsArray] } From 455bf7af0d79f3e1b99d45915998847b5d17c239 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Mon, 29 Apr 2024 18:08:29 +0300 Subject: [PATCH 6/7] Improve stable sorting test checks --- .../entities/tests/sorted_state_adapter.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index f1b34c051..7f602f64a 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -360,22 +360,30 @@ describe('Sorted State Adapter', () => { const withInitialItems = sortedItemsAdapter.setAll( sortedItemsAdapter.getInitialState(), [ + { id: 'C', order: 3, ts: 0 }, { id: 'A', order: 1, ts: 0 }, + { id: 'F', order: 4, ts: 0 }, { id: 'B', order: 2, ts: 0 }, - { id: 'C', order: 3, ts: 0 }, { id: 'D', order: 3, ts: 0 }, { id: 'E', order: 3, ts: 0 }, ], ) - expect(withInitialItems.ids).toEqual(['A', 'B', 'C', 'D', 'E']) + expect(withInitialItems.ids).toEqual(['A', 'B', 'C', 'D', 'E', 'F']) const updated = sortedItemsAdapter.updateOne(withInitialItems, { id: 'C', changes: { ts: 5 }, }) - expect(updated.ids).toEqual(['A', 'B', 'C', 'D', 'E']) + expect(updated.ids).toEqual(['A', 'B', 'C', 'D', 'E', 'F']) + + const updated2 = sortedItemsAdapter.updateOne(withInitialItems, { + id: 'D', + changes: { ts: 6 }, + }) + + expect(updated2.ids).toEqual(['A', 'B', 'C', 'D', 'E', 'F']) }) it('should let you update many entities by id in the state', () => { From 0ca3c608f9ae09a15124501aafa309e05a7db56a Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Wed, 8 May 2024 21:12:19 -0400 Subject: [PATCH 7/7] Clean up dead code --- .../src/entities/sorted_state_adapter.ts | 457 +----------------- .../tests/sorted_state_adapter.test.ts | 16 +- packages/toolkit/src/entities/utils.ts | 1 - 3 files changed, 25 insertions(+), 449 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index e70bca548..8f16ed294 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -16,6 +16,7 @@ import { getCurrent, } from './utils' +// Borrowed from Replay export function findInsertIndex( sortedItems: T[], item: T, @@ -27,7 +28,6 @@ export function findInsertIndex( let middleIndex = (lowIndex + highIndex) >>> 1 const currentItem = sortedItems[middleIndex] const res = comparisonFunction(item, currentItem) - // console.log('Res: ', item, currentItem, res) if (res >= 0) { lowIndex = middleIndex + 1 } else { @@ -75,7 +75,6 @@ export function createSortedStateAdapter( ) const models = newEntities.filter( - // (model) => !(selectId(model) in state.entities), (model) => !existingKeys.has(selectIdValue(model, selectId)), ) @@ -94,7 +93,6 @@ export function createSortedStateAdapter( ): void { newEntities = ensureEntitiesArray(newEntities) if (newEntities.length !== 0) { - //const updatedIds = new Set(newEntities.map((item) => selectId(item))) for (const item of newEntities) { delete (state.entities as Record)[selectId(item)] } @@ -123,7 +121,6 @@ export function createSortedStateAdapter( ): void { let appliedUpdates = false let replacedIds = false - const updatedIds = new Set() for (let update of updates) { const entity: T | undefined = (state.entities as Record)[update.id] @@ -135,11 +132,12 @@ export function createSortedStateAdapter( Object.assign(entity, update.changes) const newId = selectId(entity) - updatedIds.add(newId) + if (update.id !== newId) { + // We do support the case where updates can change an item's ID. + // This makes things trickier - go ahead and swap the IDs in state now. replacedIds = true delete (state.entities as Record)[update.id] - updatedIds.delete(update.id) const oldIndex = (state.ids as Id[]).indexOf(update.id) state.ids[oldIndex] = newId ;(state.entities as Record)[newId] = entity @@ -147,8 +145,7 @@ export function createSortedStateAdapter( } if (appliedUpdates) { - mergeFunction(state, [], updatedIds, replacedIds) - // resortEntities(state, [], replacedIds) + mergeFunction(state, [], appliedUpdates, replacedIds) } } @@ -188,168 +185,29 @@ export function createSortedStateAdapter( return true } - function merge(models: readonly T[], state: R): void { - // Insert/overwrite all new/updated - models.forEach((model) => { - ;(state.entities as Record)[selectId(model)] = model - }) - - resortEntities(state, models) - } - - function cleanItem(item: T) { - return isDraft(item) ? current(item) : item - } - type MergeFunction = ( state: R, addedItems: readonly T[], - updatedIds?: Set, + appliedUpdates?: boolean, replacedIds?: boolean, ) => void - // const mergeFunction: MergeFunction = (state, addedItems) => { - // const actualMergeFunction: MergeFunction = mergeOriginal - // console.log('Merge function: ', actualMergeFunction.name) - // actualMergeFunction(state, addedItems) - // } - - const mergeOriginal: MergeFunction = (state, addedItems) => { - // Insert/overwrite all new/updated - addedItems.forEach((model) => { - ;(state.entities as Record)[selectId(model)] = model - }) - resortEntities(state) - - function resortEntities(state: R) { - const allEntities = Object.values(state.entities) as T[] - allEntities.sort(comparer) - const newSortedIds = allEntities.map(selectId) - const { ids } = state - if (!areArraysEqual(ids, newSortedIds)) { - state.ids = newSortedIds - } - } - } - - const mergeLenz: MergeFunction = ( - state, - addedItems: readonly T[], - updatedIds, - replacedIds, - ) => { - const entities = state.entities as Record - let ids = state.ids as Id[] - if (replacedIds) { - ids = Array.from(new Set(ids)) - } - const oldEntities = ids // Array.from(new Set(state.ids as Id[])) - .map((id) => entities[id]) - .filter(Boolean) - - let newSortedIds: Id[] = [] - const seenIds = new Set() - - if (addedItems.length) { - const newEntities = addedItems.slice().sort(comparer) - - // Insert/overwrite all new/updated - newEntities.forEach((model) => { - entities[selectId(model)] = model - }) - - let o = 0, - n = 0 - while (o < oldEntities.length && n < newEntities.length) { - const oldEntity = oldEntities[o] as T, - oldId = selectId(oldEntity), - newEntity = newEntities[n], - newId = selectId(newEntity) - - if (seenIds.has(newId)) { - n++ - continue - } - - const comparison = comparer(oldEntity, newEntity) - if (comparison < 0) { - // Sort the existing item first - newSortedIds.push(oldId) - seenIds.add(oldId) - o++ - continue - } - - if (comparison > 0) { - // Sort the new item first - newSortedIds.push(newId) - seenIds.add(newId) - n++ - continue - } - // The items are equivalent. Maintain stable sorting by - // putting the existing item first. - newSortedIds.push(oldId) - seenIds.add(oldId) - o++ - newSortedIds.push(newId) - seenIds.add(newId) - n++ - } - // Add any remaining existing items - while (o < oldEntities.length) { - newSortedIds.push(selectId(oldEntities[o])) - o++ - } - // Add any remaining new items - while (n < newEntities.length) { - newSortedIds.push(selectId(newEntities[n])) - n++ - } - } else if (updatedIds) { - oldEntities.sort(comparer) - newSortedIds = oldEntities.map(selectId) - } - - if (!areArraysEqual(state.ids, newSortedIds)) { - state.ids = newSortedIds - } - } - const mergeInsertion: MergeFunction = ( state, addedItems, - updatedIds, + appliedUpdates, replacedIds, ) => { const currentEntities = getCurrent(state.entities) as Record const currentIds = getCurrent(state.ids) as Id[] - // const entities = state.entities as Record + const stateEntities = state.entities as Record - // let ids = state.ids as Id[] + let ids = currentIds if (replacedIds) { ids = Array.from(new Set(currentIds)) } - // //let sortedEntities: T[] = [] - - // let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) - // .map((id) => entities[id]) - // .filter(Boolean) - - // if (addedItems.length) { - // if (wasPreviouslyEmpty) { - // sortedEntities = addedItems.slice().sort() - // } - - // for (const item of addedItems) { - // entities[selectId(item)] = item - // if (!wasPreviouslyEmpty) { - // insert(sortedEntities, item, sort) - // } - // } - // } let sortedEntities: T[] = [] for (const id of ids) { const entity = currentEntities[id] @@ -357,320 +215,35 @@ export function createSortedStateAdapter( sortedEntities.push(entity) } } - // let sortedEntities = ids // Array.from(new Set(state.ids as Id[])) - // .map((id) => currentEntities[id]) - // .filter(Boolean) - const wasPreviouslyEmpty = sortedEntities.length === 0 - // let oldIds = state.ids as Id[] - // // if (updatedIds) { - // // oldIds = oldIds.filter((id) => !updatedIds.has(id)) - // // const updatedItems = Array.from(updatedIds) - // // .map((id) => entities[id]) - // // .filter(Boolean) - // // models = updatedItems.concat(models) - // // } - // // console.log('Old IDs: ', oldIds) - // const sortedEntities = oldIds - // .map((id) => (state.entities as Record)[id as Id]) - // .filter(Boolean) - // Insert/overwrite all new/updated for (const item of addedItems) { stateEntities[selectId(item)] = item - // console.log('Inserting: ', isDraft(item) ? current(item) : item) + if (!wasPreviouslyEmpty) { + // Binary search insertion generally requires fewer comparisons insert(sortedEntities, item, comparer) } } if (wasPreviouslyEmpty) { + // All we have is the incoming values, sort them sortedEntities = addedItems.slice().sort(comparer) - } else if (updatedIds?.size) { - for (const updatedId of updatedIds) { - const item: T = currentEntities[updatedId] - // const currentIndex = sortedEntities.indexOf(item) - // const newIndex = findInsertIndex(sortedEntities, item, comparer) - // console.log('Item: ', updatedId, { currentIndex, newIndex }) - } + } else if (appliedUpdates) { + // We should have a _mostly_-sorted array already sortedEntities.sort(comparer) } const newSortedIds = sortedEntities.map(selectId) - // console.log('New sorted IDs: ', newSortedIds) - if (!areArraysEqual(currentIds, newSortedIds)) { - state.ids = newSortedIds - } - } - - const mergeInitialPR: MergeFunction = ( - state, - addedItems, - updatedIds, - replacedIds, - ) => { - // Insert/overwrite all new/updated - addedItems.forEach((model) => { - ;(state.entities as Record)[selectId(model)] = model - }) - - let allEntities: T[] - - if (replacedIds) { - // This is a really annoying edge case. Just figure this out from scratch - // rather than try to be clever. This will be more expensive since it isn't sorted right. - allEntities = Object.values(state.entities) as T[] - } else { - // We're starting with an already-sorted list. - let existingIds = state.ids - - if (addedItems.length) { - // There's a couple edge cases where we can have duplicate item IDs. - // Ensure we don't have duplicates. - const uniqueIds = new Set(existingIds as Id[]) - - addedItems.forEach((item) => { - uniqueIds.add(selectId(item)) - }) - existingIds = Array.from(uniqueIds) - } - // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. - // Make this a sorta-mostly-sorted array. - allEntities = existingIds.map( - (id) => (state.entities as Record)[id as Id], - ) - } - - // Now when we sort, things should be _close_ already, and fewer comparisons are needed. - allEntities.sort(comparer) - - const newSortedIds = allEntities.map(selectId) - const { ids } = state - - if (!areArraysEqual(ids, newSortedIds)) { - state.ids = newSortedIds - } - } - - const mergeTweakedPR: MergeFunction = ( - state, - addedItems, - updatedIds, - replacedIds, - ) => { - let allEntities: T[] - - let existingIds = state.ids - const uniqueIds = new Set(existingIds as Id[]) - existingIds = Array.from(uniqueIds) - - if (addedItems.length) { - // There's a couple edge cases where we can have duplicate item IDs. - // Ensure we don't have duplicates. - - addedItems.forEach((item) => { - ;(state.entities as Record)[selectId(item)] = item - uniqueIds.add(selectId(item)) - }) - existingIds = Array.from(uniqueIds) - } - - // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. - // Make this a sorta-mostly-sorted array. - allEntities = existingIds.map( - (id) => (state.entities as Record)[id as Id], - ) - - // if (replacedIds) { - // // There's a couple edge cases where we can have duplicate item IDs. - // // Ensure we don't have duplicates. - // const uniqueIds = new Set(existingIds as Id[]) - // // This is a really annoying edge case. Just figure this out from scratch - // // rather than try to be clever. This will be more expensive since it isn't sorted right. - // allEntities = state.ids.map( - // (id) => (state.entities as Record)[id as Id], - // ) - // } else { - // // We're starting with an already-sorted list. - // let existingIds = state.ids - - // if (addedItems.length) { - // // There's a couple edge cases where we can have duplicate item IDs. - // // Ensure we don't have duplicates. - // const uniqueIds = new Set(existingIds as Id[]) - - // addedItems.forEach((item) => { - - // ;(state.entities as Record)[selectId(item)] = item - // uniqueIds.add(selectId(item)) - // }) - // existingIds = Array.from(uniqueIds) - // } - - // // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. - // // Make this a sorta-mostly-sorted array. - // allEntities = existingIds.map( - // (id) => (state.entities as Record)[id as Id], - // ) - // } - - // Now when we sort, things should be _close_ already, and fewer comparisons are needed. - allEntities.sort(comparer) - - const newSortedIds = allEntities.map(selectId) - const { ids } = state - - if (!areArraysEqual(ids, newSortedIds)) { - state.ids = newSortedIds - } - } - - const mergeJackman: MergeFunction = ( - state, - addedItems, - updatedIds, - replacedIds, - ) => { - const entities = state.entities as Record - let ids = state.ids as Id[] - if (replacedIds) { - ids = Array.from(new Set(ids)) - } - const existingSortedItems = ids // Array.from(new Set(state.ids as Id[])) - .map((id) => entities[id]) - .filter(Boolean) - - function findInsertIndex2( - sortedItems: T[], - item: T, - comparisonFunction: Comparer, - lowIndexOverride?: number, - ): number { - let lowIndex = lowIndexOverride ?? 0 - let highIndex = sortedItems.length - while (lowIndex < highIndex) { - const middleIndex = (lowIndex + highIndex) >>> 1 - const currentItem = sortedItems[middleIndex] - if (comparisonFunction(item, currentItem) > 0) { - lowIndex = middleIndex + 1 - } else { - highIndex = middleIndex - } - } - - return lowIndex - } - - if (addedItems.length) { - const newEntities = addedItems.slice().sort(comparer) - - // Insert/overwrite all new/updated - newEntities.forEach((model) => { - entities[selectId(model)] = model - }) - - const firstInstanceId = newEntities[0] - const lastInstanceId = newEntities[newEntities.length - 1] - - const startIndex = findInsertIndex2( - existingSortedItems, - firstInstanceId, - comparer, - ) - const endIndex = findInsertIndex2( - existingSortedItems, - lastInstanceId, - comparer, - startIndex, - ) - - const overlappingExistingIds = existingSortedItems.slice( - startIndex, - endIndex, - ) - let newIdIndexOfLastInsert = 0 - let lastRelativeInsertIndex = 0 - for (let i = 1; i < newEntities.length; i++) { - const relativeInsertIndex = findInsertIndex2( - overlappingExistingIds, - newEntities[i], - comparer, - lastRelativeInsertIndex, - ) - if (lastRelativeInsertIndex !== relativeInsertIndex) { - const insertIndex = - startIndex + newIdIndexOfLastInsert + lastRelativeInsertIndex - const arrayToInsert = newEntities.slice(newIdIndexOfLastInsert, i) - existingSortedItems.splice(insertIndex, 0, ...arrayToInsert) - newIdIndexOfLastInsert = i - lastRelativeInsertIndex = relativeInsertIndex - } - } - existingSortedItems.splice( - startIndex + newIdIndexOfLastInsert + lastRelativeInsertIndex, - 0, - ...newEntities.slice(newIdIndexOfLastInsert), - ) - } else if (updatedIds?.size) { - existingSortedItems.sort(comparer) - } - - const newSortedIds = existingSortedItems.map(selectId) - - if (!areArraysEqual(ids, newSortedIds)) { + if (!areArraysEqual(currentIds, newSortedIds)) { state.ids = newSortedIds } } const mergeFunction: MergeFunction = mergeInsertion - function resortEntities( - state: R, - addedItems: readonly T[] = [], - replacedIds = false, - ) { - let allEntities: T[] - - allEntities = Object.values(state.entities) as T[] - if (replacedIds) { - // This is a really annoying edge case. Just figure this out from scratch - // rather than try to be clever. This will be more expensive since it isn't sorted right. - allEntities = Object.values(state.entities) as T[] - } else { - // We're starting with an already-sorted list. - let existingIds = state.ids - - if (addedItems.length) { - // There's a couple edge cases where we can have duplicate item IDs. - // Ensure we don't have duplicates. - const uniqueIds = new Set(existingIds as Id[]) - - addedItems.forEach((item) => { - uniqueIds.add(selectId(item)) - }) - existingIds = Array.from(uniqueIds) - } - - // By this point `ids` and `entities` should be 1:1, but not necessarily sorted. - // Make this a sorta-mostly-sorted array. - allEntities = existingIds.map( - (id) => (state.entities as Record)[id as Id], - ) - } - - // Now when we sort, things should be _close_ already, and fewer comparisons are needed. - allEntities.sort(comparer) - - const newSortedIds = allEntities.map(selectId) - const { ids } = state - - if (!areArraysEqual(ids, newSortedIds)) { - state.ids = newSortedIds - } - } - return { removeOne, removeMany, diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index 7f602f64a..cc1bf568d 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -653,6 +653,8 @@ describe('Sorted State Adapter', () => { numSorts = 0 + const logComparisons = false + function measureComparisons(name: string, cb: () => void) { numSorts = 0 const start = new Date().getTime() @@ -660,9 +662,11 @@ describe('Sorted State Adapter', () => { const end = new Date().getTime() const duration = end - start - console.log( - `${name}: sortComparer called ${numSorts.toLocaleString()} times in ${duration.toLocaleString()}ms`, - ) + if (logComparisons) { + console.log( + `${name}: sortComparer called ${numSorts.toLocaleString()} times in ${duration.toLocaleString()}ms`, + ) + } } const initialItems = generateItems(INITIAL_ITEMS) @@ -718,8 +722,8 @@ describe('Sorted State Adapter', () => { // These numbers will vary because of the randomness, but generally // with 10K items the old code had 200K+ sort calls, while the new code - // is around 130K sort calls. - // expect(numSorts).toBeLessThan(200_000) + // is around 13K sort calls. + expect(numSorts).toBeLessThan(20_000) const { ids } = store.getState().entity const middleItemId = ids[(ids.length / 2) | 0] @@ -776,7 +780,7 @@ describe('Sorted State Adapter', () => { ) // The old code was around 120K, the new code is around 10K. - // expect(numSorts).toBeLessThan(25_000) + //expect(numSorts).toBeLessThan(25_000) }) describe('can be used mutably when wrapped in createNextState', () => { diff --git a/packages/toolkit/src/entities/utils.ts b/packages/toolkit/src/entities/utils.ts index 6ed06d518..895ef2b35 100644 --- a/packages/toolkit/src/entities/utils.ts +++ b/packages/toolkit/src/entities/utils.ts @@ -56,7 +56,6 @@ export function splitAddedUpdatedEntities( for (const entity of newEntities) { const id = selectIdValue(entity, selectId) if (existingIds.has(id)) { - // if (id in state.entities) { updated.push({ id, changes: entity }) } else { added.push(entity)