Skip to content

Commit 3191483

Browse files
authored
Optimize dev check middleware performance (#943)
1 parent f05e3c6 commit 3191483

7 files changed

+352
-62
lines changed

etc/redux-toolkit.api.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ export interface EntityStateAdapter<T> {
274274
}
275275

276276
// @public (undocumented)
277-
export function findNonSerializableValue(value: unknown, path?: ReadonlyArray<string>, isSerializable?: (value: unknown) => boolean, getEntries?: (value: unknown) => [string, any][], ignoredPaths?: string[]): NonSerializableValue | false;
277+
export function findNonSerializableValue(value: unknown, path?: string, isSerializable?: (value: unknown) => boolean, getEntries?: (value: unknown) => [string, any][], ignoredPaths?: string[]): NonSerializableValue | false;
278278

279279
export { freeze }
280280

@@ -419,6 +419,7 @@ export interface SerializableStateInvariantMiddlewareOptions {
419419
ignoredActionPaths?: string[];
420420
ignoredActions?: string[];
421421
ignoredPaths?: string[];
422+
ignoreState?: boolean;
422423
isSerializable?: (value: any) => boolean;
423424
warnAfter?: number;
424425
}

src/immutablePerfBenchmarks.ts

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
// import Benchmark from 'benchmark'
2+
import { Store, MiddlewareAPI, Dispatch } from 'redux'
3+
import faker from 'faker'
4+
5+
import { configureStore } from './configureStore'
6+
import { createSlice } from './createSlice'
7+
8+
import {
9+
createImmutableStateInvariantMiddleware,
10+
tm2,
11+
ImmutableStateInvariantMiddlewareOptions
12+
} from './immutableStateInvariantMiddleware'
13+
14+
export class TaskInfo {
15+
private _taskName: string
16+
private _percentage: string | undefined
17+
private _timeMillis: number
18+
19+
constructor(taskName: string, timeMillis: number) {
20+
this._taskName = taskName
21+
this._timeMillis = timeMillis
22+
}
23+
24+
get taskName(): string {
25+
return this._taskName
26+
}
27+
28+
get timeMills(): number {
29+
return this._timeMillis
30+
}
31+
32+
get percentage(): string | undefined {
33+
return this._percentage
34+
}
35+
36+
calculatePercentage(totalTimeMillis: number): string {
37+
this._percentage = ((this._timeMillis * 100) / totalTimeMillis).toFixed(2)
38+
return this._percentage
39+
}
40+
}
41+
42+
export class StopWatch {
43+
public static NoTaskMessage = 'No task info kept'
44+
45+
private id: string
46+
private currentTaskName: string | null = null
47+
private startTimeMillis = 0
48+
private totalTimeMillis = 0
49+
private taskList: Array<TaskInfo> = []
50+
51+
constructor(id = '') {
52+
this.id = id
53+
}
54+
55+
/**
56+
* start a task
57+
*/
58+
start(taskName = ''): void {
59+
this.currentTaskName !== null &&
60+
this.throwError("Can't start StopWatch: it's already running")
61+
this.currentTaskName = taskName
62+
this.startTimeMillis = Date.now()
63+
}
64+
65+
/**
66+
* stop the current task
67+
*/
68+
stop(): void {
69+
this.currentTaskName === null &&
70+
this.throwError("Can't stop StopWatch: it's not running")
71+
const lastTime: number = Date.now() - this.startTimeMillis
72+
this.totalTimeMillis += lastTime
73+
const lastTaskInfo = new TaskInfo(this.currentTaskName!, lastTime)
74+
this.taskList.push(lastTaskInfo)
75+
this.currentTaskName = null
76+
}
77+
78+
/**
79+
* Return a string with a table describing all tasks performed.
80+
*/
81+
prettyPrint(): string {
82+
const output: Array<string> = [this.shortSummary()]
83+
if (this.taskList.length) {
84+
output.push('------------------------------------------')
85+
output.push('ms \t\t % \t\t Task name')
86+
output.push('------------------------------------------')
87+
this.taskList.forEach((task: TaskInfo) => {
88+
let percentage = '0'
89+
try {
90+
percentage = task.calculatePercentage(this.totalTimeMillis)
91+
} catch (e) {}
92+
output.push(
93+
`${task.timeMills} \t\t ${percentage} \t\t ${task.taskName}`
94+
)
95+
})
96+
} else {
97+
output.push(StopWatch.NoTaskMessage)
98+
}
99+
const outputString = output.join('\n')
100+
101+
console.info(outputString)
102+
return outputString
103+
}
104+
105+
/**
106+
* Return a task matching the given name
107+
*/
108+
getTask(taskName: string): TaskInfo | undefined {
109+
const task = this.taskList.find(task => task.taskName === taskName)
110+
if (task) {
111+
task.calculatePercentage(this.totalTimeMillis)
112+
}
113+
114+
return task
115+
}
116+
117+
/**
118+
* Return the total running time in milliseconds
119+
*/
120+
getTotalTime(): number {
121+
return this.totalTimeMillis
122+
}
123+
124+
/**
125+
* Return a short description of the total running time.
126+
*/
127+
shortSummary(): string {
128+
return `StopWatch '${this.id}' running time (millis) = ${this.totalTimeMillis}`
129+
}
130+
131+
/**
132+
* Return whether the stop watch is currently running
133+
*/
134+
isRunning(): boolean {
135+
return this.currentTaskName !== null
136+
}
137+
138+
/**
139+
* Return the number of tasks timed.
140+
*/
141+
getTaskCount(): number {
142+
return this.taskList.length
143+
}
144+
145+
private throwError(msg: string): never {
146+
throw new Error(msg)
147+
}
148+
}
149+
150+
/*
151+
let state: any
152+
const getState: Store['getState'] = () => state
153+
154+
function middleware(options: ImmutableStateInvariantMiddlewareOptions = {}) {
155+
return createImmutableStateInvariantMiddleware(options)({
156+
getState
157+
} as MiddlewareAPI)
158+
}
159+
160+
const next: Dispatch = action => action
161+
*/
162+
163+
function createSliceData() {
164+
const people = Array.from({ length: 10000 }).map(
165+
() => faker.helpers.userCard() as any
166+
)
167+
people.forEach(person => {
168+
person.vehicles = Array.from({ length: 2 }).map(() =>
169+
faker.vehicle.vehicle()
170+
)
171+
})
172+
173+
return people
174+
}
175+
176+
const state: any = {
177+
a: createSliceData(),
178+
b: createSliceData(),
179+
c: createSliceData()
180+
}
181+
182+
// debugger
183+
// let q = 42
184+
185+
const dummySlice = createSlice({
186+
name: 'dummy',
187+
initialState: state,
188+
reducers: {}
189+
})
190+
191+
const originalStore = configureStore({
192+
reducer: dummySlice.reducer,
193+
middleware: gdm =>
194+
gdm({
195+
// serializableCheck: false
196+
})
197+
})
198+
199+
function runOriginal() {
200+
// const dispatch = middleware()(next)
201+
// dispatch({ type: 'SOME_ACTION' })
202+
originalStore.dispatch({ type: 'SOME_ACTION' })
203+
}
204+
205+
const queuedStore = configureStore({
206+
reducer: dummySlice.reducer,
207+
middleware: gdm =>
208+
gdm({
209+
// serializableCheck: false,
210+
immutableCheck: {
211+
trackFunction: tm2
212+
}
213+
})
214+
})
215+
216+
function runQueued() {
217+
queuedStore.dispatch({ type: 'SOME_ACTION' })
218+
}
219+
/*
220+
const suite = new Benchmark.Suite('abcd', {
221+
setup() {
222+
state = {
223+
a: createSliceData(),
224+
b: createSliceData(),
225+
c: createSliceData()
226+
}
227+
}
228+
})
229+
230+
suite
231+
.add('Original', )
232+
.add('Queued', )
233+
.on('cycle', function(event: any) {
234+
console.log(String(event.target))
235+
})
236+
.on('complete', function(this: any) {
237+
console.log('Fastest is ' + this.filter('fastest').map('name'))
238+
})
239+
.run({})
240+
*/
241+
242+
const stopwatch = new StopWatch()
243+
244+
stopwatch.start('Original')
245+
runOriginal()
246+
stopwatch.stop()
247+
248+
stopwatch.start('Queued')
249+
runQueued()
250+
stopwatch.stop()
251+
252+
// debugger
253+
254+
stopwatch.prettyPrint()
255+
256+
// let z = q

src/immutableStateInvariantMiddleware.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ describe('trackForMutations', () => {
184184

185185
expect(tracker.detectMutations()).toEqual({
186186
wasMutated: true,
187-
path: spec.path
187+
path: spec.path.join('.')
188188
})
189189
})
190190
}

src/immutableStateInvariantMiddleware.ts

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ function getSerialize(
6868
*/
6969
export function isImmutableDefault(value: unknown): boolean {
7070
return (
71-
typeof value !== 'object' || value === null || typeof value === 'undefined'
71+
typeof value !== 'object' ||
72+
value === null ||
73+
typeof value === 'undefined' ||
74+
Object.isFrozen(value)
7275
)
7376
}
7477

@@ -94,19 +97,16 @@ function trackProperties(
9497
isImmutable: IsImmutableFunc,
9598
ignorePaths: IgnorePaths = [],
9699
obj: Record<string, any>,
97-
path: string[] = []
100+
path: string = ''
98101
) {
99102
const tracked: Partial<TrackedProperty> = { value: obj }
100103

101104
if (!isImmutable(obj)) {
102105
tracked.children = {}
103106

104107
for (const key in obj) {
105-
const childPath = path.concat(key)
106-
if (
107-
ignorePaths.length &&
108-
ignorePaths.indexOf(childPath.join('.')) !== -1
109-
) {
108+
const childPath = path ? path + '.' + key : key
109+
if (ignorePaths.length && ignorePaths.indexOf(childPath) !== -1) {
110110
continue
111111
}
112112

@@ -129,8 +129,8 @@ function detectMutations(
129129
trackedProperty: TrackedProperty,
130130
obj: any,
131131
sameParentRef: boolean = false,
132-
path: string[] = []
133-
): { wasMutated: boolean; path?: string[] } {
132+
path: string = ''
133+
): { wasMutated: boolean; path?: string } {
134134
const prevObj = trackedProperty ? trackedProperty.value : undefined
135135

136136
const sameRef = prevObj === obj
@@ -145,18 +145,16 @@ function detectMutations(
145145

146146
// Gather all keys from prev (tracked) and after objs
147147
const keysToDetect: Record<string, boolean> = {}
148-
Object.keys(trackedProperty.children).forEach(key => {
148+
for (let key in trackedProperty.children) {
149149
keysToDetect[key] = true
150-
})
151-
Object.keys(obj).forEach(key => {
150+
}
151+
for (let key in obj) {
152152
keysToDetect[key] = true
153-
})
153+
}
154154

155-
const keys = Object.keys(keysToDetect)
156-
for (let i = 0; i < keys.length; i++) {
157-
const key = keys[i]
158-
const childPath = path.concat(key)
159-
if (ignorePaths.length && ignorePaths.indexOf(childPath.join('.')) !== -1) {
155+
for (let key in keysToDetect) {
156+
const childPath = path ? path + '.' + key : key
157+
if (ignorePaths.length && ignorePaths.indexOf(childPath) !== -1) {
160158
continue
161159
}
162160

@@ -251,11 +249,8 @@ export function createImmutableStateInvariantMiddleware(
251249

252250
invariant(
253251
!result.wasMutated,
254-
`A state mutation was detected between dispatches, in the path '${(
255-
result.path || []
256-
).join(
257-
'.'
258-
)}'. This may cause incorrect behavior. (https://redux.js.org/troubleshooting#never-mutate-reducer-arguments)`
252+
`A state mutation was detected between dispatches, in the path '${result.path ||
253+
''}'. This may cause incorrect behavior. (https://redux.js.org/style-guide/style-guide#do-not-mutate-state)`
259254
)
260255
})
261256

@@ -271,13 +266,10 @@ export function createImmutableStateInvariantMiddleware(
271266
result.wasMutated &&
272267
invariant(
273268
!result.wasMutated,
274-
`A state mutation was detected inside a dispatch, in the path: ${(
275-
result.path || []
276-
).join(
277-
'.'
278-
)}. Take a look at the reducer(s) handling the action ${stringify(
269+
`A state mutation was detected inside a dispatch, in the path: ${result.path ||
270+
''}. Take a look at the reducer(s) handling the action ${stringify(
279271
action
280-
)}. (https://redux.js.org/troubleshooting#never-mutate-reducer-arguments)`
272+
)}. (https://redux.js.org/style-guide/style-guide#do-not-mutate-state)`
281273
)
282274
})
283275

0 commit comments

Comments
 (0)