Skip to content
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.

Sign up for GitHub

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

Jump to bottom

perf(store): avoid going over states list every time action is dispatched #2219

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

perf(store): avoid going over states list every time action is dispatched #2219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/store/src/internal/lifecycle-state-manager.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
}))
);
}
}
}
Loading