Skip to content

Commit

Permalink
perf(store): avoid going over states list every time action is dispat…
Browse files Browse the repository at this point in the history
…ched

In this commit, I'm working on optimizing how we handle action dispatches by avoiding
repeated traversal of the `states` list. Instead, we prepare a map each time a new state
is added, allowing us to perform O(1) lookups by action type in the future. This approach
reduces complexity and improves performance.

I've tested it with benchmark.js, and here are the results:

```
class Increment {
  static readonly type = 'Increment';
}

const states = Array.from({ length: 50 }).map((_, index) => {
  @State({
    name: `counter_${index + 1}`,
    defaults: 0,
  })
  @Injectable()
  class CounterState {
    @action(Increment)
    increment(ctx: StateContext<number>) {
      ctx.setState((counter) => counter + 1);
    }
  }

  return CounterState;
});

store.dispatch() before changes x 3,435 ops/sec ±0.45% (65 runs sampled)
store.dispatch() after changes x 3,942 ops/sec ±1.21% (25 runs sampled)
```
  • Loading branch information
arturovt committed Sep 17, 2024
1 parent bd43d33 commit 8a7cca6
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 59 deletions.
2 changes: 1 addition & 1 deletion packages/store/src/internal/lifecycle-state-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,6 @@ export class LifecycleStateManager implements OnDestroy {
}

private _getStateContext(mappedStore: MappedStore): StateContext<any> {
return this._stateContextFactory.createStateContext(mappedStore);
return this._stateContextFactory.createStateContext(mappedStore.path);
}
}
12 changes: 6 additions & 6 deletions packages/store/src/internal/state-context-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ExistingState, StateOperator, isStateOperator } from '@ngxs/store/opera
import { Observable } from 'rxjs';

import { StateContext } from '../symbols';
import { MappedStore, StateOperations } from '../internal/internals';
import { StateOperations } from '../internal/internals';
import { InternalStateOperations } from '../internal/state-operations';
import { simplePatch } from './state-operators';

Expand All @@ -19,25 +19,25 @@ export class StateContextFactory {
/**
* Create the state context
*/
createStateContext<T>(mappedStore: MappedStore): StateContext<T> {
createStateContext<T>(path: string): StateContext<T> {
const root = this._internalStateOperations.getRootStateOperations();

return {
getState(): T {
const currentAppState = root.getState();
return getState(currentAppState, mappedStore.path);
return getState(currentAppState, path);
},
patchState(val: Partial<T>): void {
const currentAppState = root.getState();
const patchOperator = simplePatch<T>(val);
setStateFromOperator(root, currentAppState, patchOperator, mappedStore.path);
setStateFromOperator(root, currentAppState, patchOperator, path);
},
setState(val: T | StateOperator<T>): void {
const currentAppState = root.getState();
if (isStateOperator(val)) {
setStateFromOperator(root, currentAppState, val, mappedStore.path);
setStateFromOperator(root, currentAppState, val, path);
} else {
setStateValue(root, currentAppState, val, mappedStore.path);
setStateValue(root, currentAppState, val, path);
}
},
dispatch(actions: any | any[]): Observable<void> {
Expand Down
145 changes: 93 additions & 52 deletions packages/store/src/internal/state-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
ɵStateClassInternal,
ɵINITIAL_STATE_TOKEN,
ɵSharedSelectorOptions,
ɵRuntimeSelectorContext
ɵRuntimeSelectorContext,
ɵActionHandlerMetaData
} from '@ngxs/store/internals';
import { getActionTypeFromInstance, getValue, setValue } from '@ngxs/store/plugins';
import {
Expand Down Expand Up @@ -78,6 +79,11 @@ function cloneDefaults(defaults: any): any {
return value;
}

interface InvokableActionHandlerMetaData extends ɵActionHandlerMetaData {
path: string;
instance: any;
}

/**
* The `StateFactory` class adds root and feature states to the graph.
* This extracts state names from state classes, checks if they already
Expand Down Expand Up @@ -113,6 +119,14 @@ export class StateFactory implements OnDestroy {
private _initialState: any
) {}

private _actionTypeToMetasMap = new Map<string, InvokableActionHandlerMetaData[]>();

get actionTypeToMetasMap(): Map<string, InvokableActionHandlerMetaData[]> {
return this._parentFactory
? this._parentFactory.actionTypeToMetasMap
: this._actionTypeToMetasMap;
}

private _states: MappedStore[] = [];

get states(): MappedStore[] {
Expand Down Expand Up @@ -223,6 +237,7 @@ export class StateFactory implements OnDestroy {
}

this.states.push(stateMap);
this.hydrateActionMetasMap(stateMap);
}

return bootstrappedStores;
Expand Down Expand Up @@ -290,64 +305,63 @@ export class StateFactory implements OnDestroy {
// to `true` within the below `for` loop if any `actionMetas` has been found.
let actionHasBeenHandled = false;

for (const metadata of this.states) {
const actionMetas = metadata.actions[type];

if (actionMetas) {
for (const actionMeta of actionMetas) {
const stateContext = this._stateContextFactory.createStateContext(metadata);
try {
let result = metadata.instance[actionMeta.fn](stateContext, action);

// We need to use `isPromise` instead of checking whether
// `result instanceof Promise`. In zone.js patched environments, `global.Promise`
// is the `ZoneAwarePromise`. Some APIs, which are likely not patched by zone.js
// for certain reasons, might not work with `instanceof`. For instance, the dynamic
// import returns a native promise (not a `ZoneAwarePromise`), causing this check to
// be falsy.
if (ɵisPromise(result)) {
result = from(result);
}
const actionMetas = this.actionTypeToMetasMap.get(type);

if (actionMetas) {
for (const actionMeta of actionMetas) {
const stateContext = this._stateContextFactory.createStateContext(actionMeta.path);

try {
let result = actionMeta.instance[actionMeta.fn](stateContext, action);

// We need to use `isPromise` instead of checking whether
// `result instanceof Promise`. In zone.js patched environments, `global.Promise`
// is the `ZoneAwarePromise`. Some APIs, which are likely not patched by zone.js
// for certain reasons, might not work with `instanceof`. For instance, the dynamic
// import returns a native promise (not a `ZoneAwarePromise`), causing this check to
// be falsy.
if (ɵisPromise(result)) {
result = from(result);
}

if (isObservable(result)) {
// If this observable has been completed w/o emitting
// any value then we wouldn't want to complete the whole chain
// of actions. Since if any observable completes then
// action will be canceled.
// For instance if any action handler would've had such statement:
// `handler(ctx) { return EMPTY; }`
// then the action will be canceled.
// See https://github.com/ngxs/store/issues/1568
if (isObservable(result)) {
// If this observable has been completed w/o emitting
// any value then we wouldn't want to complete the whole chain
// of actions. Since if any observable completes then
// action will be canceled.
// For instance if any action handler would've had such statement:
// `handler(ctx) { return EMPTY; }`
// then the action will be canceled.
// See https://github.com/ngxs/store/issues/1568
result = result.pipe(
mergeMap((value: any) => {
if (ɵisPromise(value)) {
return from(value);
}
if (isObservable(value)) {
return value;
}
return of(value);
}),
defaultIfEmpty({})
);

if (actionMeta.options.cancelUncompleted) {
// todo: ofActionDispatched should be used with action class
result = result.pipe(
mergeMap((value: any) => {
if (ɵisPromise(value)) {
return from(value);
}
if (isObservable(value)) {
return value;
}
return of(value);
}),
defaultIfEmpty({})
takeUntil(dispatched$.pipe(ofActionDispatched(action as any)))
);

if (actionMeta.options.cancelUncompleted) {
// todo: ofActionDispatched should be used with action class
result = result.pipe(
takeUntil(dispatched$.pipe(ofActionDispatched(action as any)))
);
}
} else {
result = of({}).pipe(shareReplay());
}

results.push(result);
} catch (e) {
results.push(throwError(e));
} else {
result = of({}).pipe(shareReplay());
}

actionHasBeenHandled = true;
results.push(result);
} catch (e) {
results.push(throwError(e));
}

actionHasBeenHandled = true;
}
}

Expand Down Expand Up @@ -406,4 +420,31 @@ export class StateFactory implements OnDestroy {
// its lifecycle is in 'bootstrapped' state.
return this.statesByName[name] && valueIsBootstrappedInInitialState;
}

private hydrateActionMetasMap({ path, actions, instance }: MappedStore): void {
const actionTypeToMetasMap = this.actionTypeToMetasMap;

for (const actionType of Object.keys(actions)) {
// Initialize the map entry if it does not already exist for that
// action type. Note that action types may overlap between states,
// as the same action can be handled by different states.
if (!actionTypeToMetasMap.has(actionType)) {
actionTypeToMetasMap.set(actionType, []);
}

const extendedActionMetas = actionTypeToMetasMap.get(actionType)!;

extendedActionMetas.push(
// This involves combining each individual action metadata with
// the state instance and the path—essentially everything needed
// to invoke an action. This eliminates the need to loop over states
// every time an action is dispatched.
...actions[actionType].map(actionMeta => ({
...actionMeta,
path,
instance
}))
);
}
}
}

0 comments on commit 8a7cca6

Please sign in to comment.