Skip to content

Commit fdb3ab9

Browse files
feat(StateRegistry): improve perf for: .register() and StateMatcher.find() misses
The `.find()` function is pretty hot/frequently used. When possible, a state is found using a property look up `this._states[name]` If a state isn't found using an exact match, another attempt is made to do a glob match. The glob match allows a future state such as `future.state.**` to be matched for any child such as `future.state.child.nested`. This causes unnecessary overhead especially during bootup and registration. Before registering a state, we first check to see if it already exists. This change does three things: 1) Only state names that *might* be a glob are tested 2) The Globs for states with glob-like names are cached 3) Do not use globs in `.register()` when checking for "State foo is already defined" Related to #27 Related to angular-ui/ui-router#3274
1 parent 4193244 commit fdb3ab9

File tree

6 files changed

+42
-25
lines changed

6 files changed

+42
-25
lines changed

src/common/glob.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,13 @@ export class Glob {
7373
return this.regexp.test('.' + name);
7474
}
7575

76-
/** @deprecated whats the point? */
76+
/** Returns true if the string has glob-like characters in it */
7777
static is(text: string) {
78-
return text.indexOf('*') > -1;
78+
return !!/[!,*]+/.exec(text);
7979
}
8080

81-
/** @deprecated whats the point? */
81+
/** Returns a glob from the string, or null if the string isn't Glob-like */
8282
static fromString(text: string) {
83-
if (!this.is(text)) return null;
84-
return new Glob(text);
83+
return Glob.is(text) ? new Glob(text) : null;
8584
}
8685
}

src/state/stateMatcher.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import {isString} from "../common/predicates";
33
import {StateOrName} from "./interface";
44
import {State} from "./stateObject";
5-
import {Glob} from "../common/glob";
65
import {values} from "../common/common";
76

87
export class StateMatcher {
@@ -25,8 +24,11 @@ export class StateMatcher {
2524
if (state && (isStr || (!isStr && (state === stateOrName || state.self === stateOrName)))) {
2625
return state;
2726
} else if (isStr) {
28-
let matches = values(this._states)
29-
.filter(state => new Glob(state.name).matches(name));
27+
let _states = values(this._states);
28+
let matches = _states.filter(state =>
29+
state.__stateObjectCache.nameGlob &&
30+
state.__stateObjectCache.nameGlob.matches(name)
31+
);
3032

3133
if (matches.length > 1) {
3234
console.log(`stateMatcher.find: Found multiple matches for ${name} using glob: `, matches.map(match => match.name));

src/state/stateObject.ts

+13-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { Resolvable } from "../resolve/resolvable";
1212
import { TransitionStateHookFn } from "../transition/interface";
1313
import { TargetState } from "./targetState";
1414
import { Transition } from "../transition/transition";
15+
import { Glob } from "../common/glob";
16+
import { isObject } from "../common/predicates";
1517

1618
/**
1719
* Internal representation of a UI-Router state.
@@ -95,6 +97,12 @@ export class State {
9597
{ state: (string|StateDeclaration), params: { [key: string]: any }}
9698
);
9799

100+
/** @hidden */
101+
__stateObjectCache: {
102+
/** Might be null */
103+
nameGlob?: Glob
104+
};
105+
98106

99107
/** @deprecated use State.create() */
100108
constructor(config?: StateDeclaration) {
@@ -112,14 +120,16 @@ export class State {
112120
static create(stateDecl: StateDeclaration): State {
113121
let state = inherit(inherit(stateDecl, State.prototype)) as State;
114122
stateDecl.$$state = () => state;
115-
state['__stateObject'] = true;
116123
state.self = stateDecl;
124+
state.__stateObjectCache = {
125+
nameGlob: Glob.fromString(state.name) // might return null
126+
};
117127
return state;
118128
}
119129

120-
/** Predicate which returns true if the object is an internal State object */
130+
/** Predicate which returns true if the object is an internal [[State]] object */
121131
static isState = (obj: any): obj is State =>
122-
obj['__stateObject'] === true;
132+
isObject(obj['__stateObjectCache']);
123133

124134
/**
125135
* Returns true if the provided parameter is the same state.

src/state/stateQueueManager.ts

+18-12
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
/** @module state */ /** for typedoc */
2-
import { extend, inherit, pluck, inArray } from "../common/common";
3-
import { isString, isDefined } from "../common/predicates";
2+
import { inArray } from "../common/common";
3+
import { isString } from "../common/predicates";
44
import { StateDeclaration } from "./interface";
55
import { State } from "./stateObject";
66
import { StateBuilder } from "./stateBuilder";
77
import { StateRegistryListener, StateRegistry } from "./stateRegistry";
88
import { Disposable } from "../interface";
99
import { UrlRouter } from "../url/urlRouter";
1010
import { prop } from "../common/hof";
11+
import { StateMatcher } from "./stateMatcher";
1112

1213
/** @internalapi */
1314
export class StateQueueManager implements Disposable {
1415
queue: State[];
16+
matcher: StateMatcher;
1517

1618
constructor(
1719
private $registry: StateRegistry,
@@ -20,6 +22,7 @@ export class StateQueueManager implements Disposable {
2022
public builder: StateBuilder,
2123
public listeners: StateRegistryListener[]) {
2224
this.queue = [];
25+
this.matcher = $registry.matcher;
2326
}
2427

2528
/** @internalapi */
@@ -47,36 +50,39 @@ export class StateQueueManager implements Disposable {
4750
let registered: State[] = [], // states that got registered
4851
orphans: State[] = [], // states that don't yet have a parent registered
4952
previousQueueLength = {}; // keep track of how long the queue when an orphan was first encountered
53+
const getState = (name) =>
54+
this.states.hasOwnProperty(name) && this.states[name];
5055

5156
while (queue.length > 0) {
5257
let state: State = queue.shift();
58+
let name = state.name;
5359
let result: State = builder.build(state);
5460
let orphanIdx: number = orphans.indexOf(state);
5561

5662
if (result) {
57-
let existingState = this.$registry.get(state.name);
58-
59-
if (existingState && existingState.name === state.name) {
60-
throw new Error(`State '${state.name}' is already defined`);
63+
let existingState = getState(name);
64+
if (existingState && existingState.name === name) {
65+
throw new Error(`State '${name}' is already defined`);
6166
}
6267

63-
if (existingState && existingState.name === state.name + ".**") {
68+
let existingFutureState = getState(name + ".**");
69+
if (existingFutureState) {
6470
// Remove future state of the same name
65-
this.$registry.deregister(existingState);
71+
this.$registry.deregister(existingFutureState);
6672
}
6773

68-
states[state.name] = state;
74+
states[name] = state;
6975
this.attachRoute(state);
7076
if (orphanIdx >= 0) orphans.splice(orphanIdx, 1);
7177
registered.push(state);
7278
continue;
7379
}
7480

75-
let prev = previousQueueLength[state.name];
76-
previousQueueLength[state.name] = queue.length;
81+
let prev = previousQueueLength[name];
82+
previousQueueLength[name] = queue.length;
7783
if (orphanIdx >= 0 && prev === queue.length) {
7884
// Wait until two consecutive iterations where no additional states were dequeued successfully.
79-
// throw new Error(`Cannot register orphaned state '${state.name}'`);
85+
// throw new Error(`Cannot register orphaned state '${name}'`);
8086
queue.push(state);
8187
return states;
8288
} else if (orphanIdx < 0) {

src/transition/rejectFactory.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class Rejection {
2929
return `TransitionRejection(type: ${type}, message: ${message}, detail: ${detail})`;
3030
}
3131

32-
toPromise() {
32+
toPromise(): Promise<any> {
3333
return extend(silentRejection(this), { _transitionRejection: this });
3434
}
3535

src/vanilla/$injector.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export const $injector = {
7676
invoke: (fn: IInjectable, context?, locals?) => {
7777
let all = extend({}, globals, locals || {});
7878
let params = $injector.annotate(fn);
79-
let ensureExist = assertPredicate(key => all.hasOwnProperty(key), key => `DI can't find injectable: '${key}'`);
79+
let ensureExist = assertPredicate((key: string) => all.hasOwnProperty(key), key => `DI can't find injectable: '${key}'`);
8080
let args = params.filter(ensureExist).map(x => all[x]);
8181
if (isFunction(fn)) return fn.apply(context, args);
8282
else return (fn as any[]).slice(-1)[0].apply(context, args);

0 commit comments

Comments
 (0)