Skip to content

Optimize dev check middleware performance #943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion etc/redux-toolkit.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ export interface EntityStateAdapter<T> {
}

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

export { freeze }

Expand Down Expand Up @@ -419,6 +419,7 @@ export interface SerializableStateInvariantMiddlewareOptions {
ignoredActionPaths?: string[];
ignoredActions?: string[];
ignoredPaths?: string[];
ignoreState?: boolean;
isSerializable?: (value: any) => boolean;
warnAfter?: number;
}
Expand Down
256 changes: 256 additions & 0 deletions src/immutablePerfBenchmarks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
// import Benchmark from 'benchmark'
import { Store, MiddlewareAPI, Dispatch } from 'redux'
import faker from 'faker'

import { configureStore } from './configureStore'
import { createSlice } from './createSlice'

import {
createImmutableStateInvariantMiddleware,
tm2,
ImmutableStateInvariantMiddlewareOptions
} from './immutableStateInvariantMiddleware'

export class TaskInfo {
private _taskName: string
private _percentage: string | undefined
private _timeMillis: number

constructor(taskName: string, timeMillis: number) {
this._taskName = taskName
this._timeMillis = timeMillis
}

get taskName(): string {
return this._taskName
}

get timeMills(): number {
return this._timeMillis
}

get percentage(): string | undefined {
return this._percentage
}

calculatePercentage(totalTimeMillis: number): string {
this._percentage = ((this._timeMillis * 100) / totalTimeMillis).toFixed(2)
return this._percentage
}
}

export class StopWatch {
public static NoTaskMessage = 'No task info kept'

private id: string
private currentTaskName: string | null = null
private startTimeMillis = 0
private totalTimeMillis = 0
private taskList: Array<TaskInfo> = []

constructor(id = '') {
this.id = id
}

/**
* start a task
*/
start(taskName = ''): void {
this.currentTaskName !== null &&
this.throwError("Can't start StopWatch: it's already running")
this.currentTaskName = taskName
this.startTimeMillis = Date.now()
}

/**
* stop the current task
*/
stop(): void {
this.currentTaskName === null &&
this.throwError("Can't stop StopWatch: it's not running")
const lastTime: number = Date.now() - this.startTimeMillis
this.totalTimeMillis += lastTime
const lastTaskInfo = new TaskInfo(this.currentTaskName!, lastTime)
this.taskList.push(lastTaskInfo)
this.currentTaskName = null
}

/**
* Return a string with a table describing all tasks performed.
*/
prettyPrint(): string {
const output: Array<string> = [this.shortSummary()]
if (this.taskList.length) {
output.push('------------------------------------------')
output.push('ms \t\t % \t\t Task name')
output.push('------------------------------------------')
this.taskList.forEach((task: TaskInfo) => {
let percentage = '0'
try {
percentage = task.calculatePercentage(this.totalTimeMillis)
} catch (e) {}
output.push(
`${task.timeMills} \t\t ${percentage} \t\t ${task.taskName}`
)
})
} else {
output.push(StopWatch.NoTaskMessage)
}
const outputString = output.join('\n')

console.info(outputString)
return outputString
}

/**
* Return a task matching the given name
*/
getTask(taskName: string): TaskInfo | undefined {
const task = this.taskList.find(task => task.taskName === taskName)
if (task) {
task.calculatePercentage(this.totalTimeMillis)
}

return task
}

/**
* Return the total running time in milliseconds
*/
getTotalTime(): number {
return this.totalTimeMillis
}

/**
* Return a short description of the total running time.
*/
shortSummary(): string {
return `StopWatch '${this.id}' running time (millis) = ${this.totalTimeMillis}`
}

/**
* Return whether the stop watch is currently running
*/
isRunning(): boolean {
return this.currentTaskName !== null
}

/**
* Return the number of tasks timed.
*/
getTaskCount(): number {
return this.taskList.length
}

private throwError(msg: string): never {
throw new Error(msg)
}
}

/*
let state: any
const getState: Store['getState'] = () => state

function middleware(options: ImmutableStateInvariantMiddlewareOptions = {}) {
return createImmutableStateInvariantMiddleware(options)({
getState
} as MiddlewareAPI)
}

const next: Dispatch = action => action
*/

function createSliceData() {
const people = Array.from({ length: 10000 }).map(
() => faker.helpers.userCard() as any
)
people.forEach(person => {
person.vehicles = Array.from({ length: 2 }).map(() =>
faker.vehicle.vehicle()
)
})

return people
}

const state: any = {
a: createSliceData(),
b: createSliceData(),
c: createSliceData()
}

// debugger
// let q = 42

const dummySlice = createSlice({
name: 'dummy',
initialState: state,
reducers: {}
})

const originalStore = configureStore({
reducer: dummySlice.reducer,
middleware: gdm =>
gdm({
// serializableCheck: false
})
})

function runOriginal() {
// const dispatch = middleware()(next)
// dispatch({ type: 'SOME_ACTION' })
originalStore.dispatch({ type: 'SOME_ACTION' })
}

const queuedStore = configureStore({
reducer: dummySlice.reducer,
middleware: gdm =>
gdm({
// serializableCheck: false,
immutableCheck: {
trackFunction: tm2
}
})
})

function runQueued() {
queuedStore.dispatch({ type: 'SOME_ACTION' })
}
/*
const suite = new Benchmark.Suite('abcd', {
setup() {
state = {
a: createSliceData(),
b: createSliceData(),
c: createSliceData()
}
}
})

suite
.add('Original', )
.add('Queued', )
.on('cycle', function(event: any) {
console.log(String(event.target))
})
.on('complete', function(this: any) {
console.log('Fastest is ' + this.filter('fastest').map('name'))
})
.run({})
*/

const stopwatch = new StopWatch()

stopwatch.start('Original')
runOriginal()
stopwatch.stop()

stopwatch.start('Queued')
runQueued()
stopwatch.stop()

// debugger

stopwatch.prettyPrint()

// let z = q
2 changes: 1 addition & 1 deletion src/immutableStateInvariantMiddleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('trackForMutations', () => {

expect(tracker.detectMutations()).toEqual({
wasMutated: true,
path: spec.path
path: spec.path.join('.')
})
})
}
Expand Down
50 changes: 21 additions & 29 deletions src/immutableStateInvariantMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ function getSerialize(
*/
export function isImmutableDefault(value: unknown): boolean {
return (
typeof value !== 'object' || value === null || typeof value === 'undefined'
typeof value !== 'object' ||
value === null ||
typeof value === 'undefined' ||
Object.isFrozen(value)
)
}

Expand All @@ -94,19 +97,16 @@ function trackProperties(
isImmutable: IsImmutableFunc,
ignorePaths: IgnorePaths = [],
obj: Record<string, any>,
path: string[] = []
path: string = ''
) {
const tracked: Partial<TrackedProperty> = { value: obj }

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

for (const key in obj) {
const childPath = path.concat(key)
if (
ignorePaths.length &&
ignorePaths.indexOf(childPath.join('.')) !== -1
) {
const childPath = path ? path + '.' + key : key
if (ignorePaths.length && ignorePaths.indexOf(childPath) !== -1) {
continue
}

Expand All @@ -129,8 +129,8 @@ function detectMutations(
trackedProperty: TrackedProperty,
obj: any,
sameParentRef: boolean = false,
path: string[] = []
): { wasMutated: boolean; path?: string[] } {
path: string = ''
): { wasMutated: boolean; path?: string } {
const prevObj = trackedProperty ? trackedProperty.value : undefined

const sameRef = prevObj === obj
Expand All @@ -145,18 +145,16 @@ function detectMutations(

// Gather all keys from prev (tracked) and after objs
const keysToDetect: Record<string, boolean> = {}
Object.keys(trackedProperty.children).forEach(key => {
for (let key in trackedProperty.children) {
keysToDetect[key] = true
})
Object.keys(obj).forEach(key => {
}
for (let key in obj) {
keysToDetect[key] = true
})
}

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

Expand Down Expand Up @@ -251,11 +249,8 @@ export function createImmutableStateInvariantMiddleware(

invariant(
!result.wasMutated,
`A state mutation was detected between dispatches, in the path '${(
result.path || []
).join(
'.'
)}'. This may cause incorrect behavior. (https://redux.js.org/troubleshooting#never-mutate-reducer-arguments)`
`A state mutation was detected between dispatches, in the path '${result.path ||
''}'. This may cause incorrect behavior. (https://redux.js.org/style-guide/style-guide#do-not-mutate-state)`
)
})

Expand All @@ -271,13 +266,10 @@ export function createImmutableStateInvariantMiddleware(
result.wasMutated &&
invariant(
!result.wasMutated,
`A state mutation was detected inside a dispatch, in the path: ${(
result.path || []
).join(
'.'
)}. Take a look at the reducer(s) handling the action ${stringify(
`A state mutation was detected inside a dispatch, in the path: ${result.path ||
''}. Take a look at the reducer(s) handling the action ${stringify(
action
)}. (https://redux.js.org/troubleshooting#never-mutate-reducer-arguments)`
)}. (https://redux.js.org/style-guide/style-guide#do-not-mutate-state)`
)
})

Expand Down
Loading