perf(ivy): remove megamorphic read from renderStringify (#30082)

The `renderStringify` function is used in a lot of performance-sensitive places, however it contains a megamorphic read which is used primarily for error messages. These changes introduce a new function that can be used to stringify output for errors and removes the megamorphic read from `renderStringify`.

This PR resolves FW-1286.

PR Close #30082
This commit is contained in:
Kristiyan Kostadinov 2019-04-24 14:50:01 +02:00 committed by Andrew Kushnir
parent 24c61cb63e
commit 2e21997016
11 changed files with 57 additions and 39 deletions

View File

@ -26,7 +26,7 @@ import {applyOnCreateInstructions} from './node_util';
import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setActiveHostElement} from './state';
import {renderInitialClasses, renderInitialStyles} from './styling/class_and_style_bindings';
import {publishDefaultGlobalUtils} from './util/global_utils';
import {defaultScheduler, renderStringify} from './util/misc_utils';
import {defaultScheduler, stringifyForError} from './util/misc_utils';
import {getRootContext} from './util/view_traversal_utils';
import {readPatchedLView, resetPreOrderHookFlags} from './util/view_utils';
@ -87,7 +87,7 @@ type HostFeature = (<T>(component: T, componentDef: ComponentDef<T>) => void);
// TODO: A hack to not pull in the NullInjector from @angular/core.
export const NULL_INJECTOR: Injector = {
get: (token: any, notFoundValue?: any) => {
throw new Error('NullInjector: Not found: ' + renderStringify(token));
throw new Error('NullInjector: Not found: ' + stringifyForError(token));
}
};

View File

@ -23,7 +23,7 @@ import {assertNodeOfPossibleTypes} from './node_assert';
import {getLView, getPreviousOrParentTNode, setTNodeAndViewData} from './state';
import {isNameOnlyAttributeMarker} from './util/attrs_utils';
import {getParentInjectorIndex, getParentInjectorView, hasParentInjector} from './util/injector_utils';
import {renderStringify} from './util/misc_utils';
import {stringifyForError} from './util/misc_utils';
import {findComponentView} from './util/view_traversal_utils';
import {isComponent, isComponentDef} from './util/view_utils';
@ -349,7 +349,7 @@ export function getOrCreateInjectable<T>(
try {
const value = bloomHash();
if (value == null && !(flags & InjectFlags.Optional)) {
throw new Error(`No provider for ${renderStringify(token)}!`);
throw new Error(`No provider for ${stringifyForError(token)}!`);
} else {
return value;
}
@ -447,7 +447,7 @@ export function getOrCreateInjectable<T>(
if (flags & InjectFlags.Optional) {
return notFoundValue;
} else {
throw new Error(`NodeInjector: NOT_FOUND [${renderStringify(token)}]`);
throw new Error(`NodeInjector: NOT_FOUND [${stringifyForError(token)}]`);
}
}
@ -545,7 +545,7 @@ export function getNodeInjectable(
if (isFactory(value)) {
const factory: NodeInjectorFactory = value;
if (factory.resolving) {
throw new Error(`Circular dep for ${renderStringify(tData[index])}`);
throw new Error(`Circular dep for ${stringifyForError(tData[index])}`);
}
const previousIncludeViewProviders = setIncludeViewProviders(factory.canSeeViewProviders);
factory.resolving = true;

View File

@ -35,7 +35,7 @@ import {initializeStaticContext as initializeStaticStylingContext} from '../styl
import {ANIMATION_PROP_PREFIX, isAnimationProp} from '../styling/util';
import {NO_CHANGE} from '../tokens';
import {attrsStylingIndexOf} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER, renderStringify} from '../util/misc_utils';
import {INTERPOLATION_DELIMITER, stringifyForError} from '../util/misc_utils';
import {getLViewParent, getRootContext} from '../util/view_traversal_utils';
import {getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isComponent, isComponentDef, isContentQueryHost, isRootView, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';
@ -673,7 +673,7 @@ function createViewBlueprint(bindingStartIndex: number, initialViewLength: numbe
}
export function createError(text: string, token: any) {
return new Error(`Renderer: ${text} [${renderStringify(token)}]`);
return new Error(`Renderer: ${text} [${stringifyForError(token)}]`);
}

View File

@ -20,7 +20,7 @@ import {getBaseDef, getComponentDef, getDirectiveDef} from '../definition';
import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
import {NG_BASE_DEF, NG_COMPONENT_DEF, NG_DIRECTIVE_DEF} from '../fields';
import {ComponentType} from '../interfaces/definition';
import {renderStringify} from '../util/misc_utils';
import {stringifyForError} from '../util/misc_utils';
import {angularCoreEnv} from './environment';
import {flushModuleScopingQueueAsMuchAsPossible, patchComponentDefWithScope, transitiveScopesFor} from './module';
@ -45,9 +45,9 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
const compiler = getCompilerFacade();
if (ngComponentDef === null) {
if (componentNeedsResolution(metadata)) {
const error = [`Component '${renderStringify(type)}' is not resolved:`];
const error = [`Component '${type.name}' is not resolved:`];
if (metadata.templateUrl) {
error.push(` - templateUrl: ${renderStringify(metadata.templateUrl)}`);
error.push(` - templateUrl: ${metadata.templateUrl}`);
}
if (metadata.styleUrls && metadata.styleUrls.length) {
error.push(` - styleUrls: ${JSON.stringify(metadata.styleUrls)}`);
@ -56,11 +56,10 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
throw new Error(error.join('\n'));
}
const templateUrl = metadata.templateUrl || `ng:///${renderStringify(type)}/template.html`;
const templateUrl = metadata.templateUrl || `ng:///${type.name}/template.html`;
const meta: R3ComponentMetadataFacade = {
...directiveMetadata(type, metadata),
typeSourceSpan:
compiler.createParseSourceSpan('Component', renderStringify(type), templateUrl),
typeSourceSpan: compiler.createParseSourceSpan('Component', type.name, templateUrl),
template: metadata.template || '',
preserveWhitespaces: metadata.preserveWhitespaces || false,
styles: metadata.styles || EMPTY_ARRAY,
@ -127,8 +126,7 @@ export function compileDirective(type: Type<any>, directive: Directive): void {
const sourceMapUrl = `ng://${name}/ngDirectiveDef.js`;
const compiler = getCompilerFacade();
const facade = directiveMetadata(type as ComponentType<any>, directive);
facade.typeSourceSpan =
compiler.createParseSourceSpan('Directive', renderStringify(type), sourceMapUrl);
facade.typeSourceSpan = compiler.createParseSourceSpan('Directive', name, sourceMapUrl);
if (facade.usesInheritance) {
addBaseDefToUndecoratedParents(type);
}
@ -269,7 +267,7 @@ function extractQueriesMetadata(
if (!ann.selector) {
throw new Error(
`Can't construct a query for the property "${field}" of ` +
`"${renderStringify(type)}" since the query selector wasn't defined.`);
`"${stringifyForError(type)}" since the query selector wasn't defined.`);
}
if (annotations.some(isInputAnn)) {
throw new Error(`Cannot combine @Input decorators with query decorators`);

View File

@ -19,7 +19,7 @@ import {getComponentDef, getDirectiveDef, getNgModuleDef, getPipeDef} from '../d
import {NG_COMPONENT_DEF, NG_DIRECTIVE_DEF, NG_MODULE_DEF, NG_PIPE_DEF} from '../fields';
import {ComponentDef} from '../interfaces/definition';
import {NgModuleType} from '../ng_module_ref';
import {maybeUnwrapFn, renderStringify} from '../util/misc_utils';
import {maybeUnwrapFn, stringifyForError} from '../util/misc_utils';
import {angularCoreEnv} from './environment';
@ -188,7 +188,7 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void {
const def = getComponentDef(type) || getDirectiveDef(type) || getPipeDef(type);
if (!def) {
errors.push(
`Unexpected value '${renderStringify(type)}' declared by the module '${renderStringify(moduleType)}'. Please add a @Pipe/@Directive/@Component annotation.`);
`Unexpected value '${stringifyForError(type)}' declared by the module '${stringifyForError(moduleType)}'. Please add a @Pipe/@Directive/@Component annotation.`);
}
}
@ -202,7 +202,7 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void {
if (combinedDeclarations.lastIndexOf(type) === -1) {
// We are exporting something which we don't explicitly declare or import.
errors.push(
`Can't export ${kind} ${renderStringify(type)} from ${renderStringify(moduleType)} as it was neither declared nor imported!`);
`Can't export ${kind} ${stringifyForError(type)} from ${stringifyForError(moduleType)} as it was neither declared nor imported!`);
}
}
}
@ -211,11 +211,11 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void {
type = resolveForwardRef(type);
const existingModule = ownerNgModule.get(type);
if (existingModule && existingModule !== moduleType) {
const modules = [existingModule, moduleType].map(renderStringify).sort();
const modules = [existingModule, moduleType].map(stringifyForError).sort();
errors.push(
`Type ${renderStringify(type)} is part of the declarations of 2 modules: ${modules[0]} and ${modules[1]}! ` +
`Please consider moving ${renderStringify(type)} to a higher module that imports ${modules[0]} and ${modules[1]}. ` +
`You can also create a new NgModule that exports and includes ${renderStringify(type)} then import that NgModule in ${modules[0]} and ${modules[1]}.`);
`Type ${stringifyForError(type)} is part of the declarations of 2 modules: ${modules[0]} and ${modules[1]}! ` +
`Please consider moving ${stringifyForError(type)} to a higher module that imports ${modules[0]} and ${modules[1]}. ` +
`You can also create a new NgModule that exports and includes ${stringifyForError(type)} then import that NgModule in ${modules[0]} and ${modules[1]}.`);
} else {
// Mark type as having owner.
ownerNgModule.set(type, moduleType);
@ -227,14 +227,14 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void {
const existingModule = ownerNgModule.get(type);
if (!existingModule) {
errors.push(
`Component ${renderStringify(type)} is not part of any NgModule or the module has not been imported into your module.`);
`Component ${stringifyForError(type)} is not part of any NgModule or the module has not been imported into your module.`);
}
}
function verifyCorrectBootstrapType(type: Type<any>) {
type = resolveForwardRef(type);
if (!getComponentDef(type)) {
errors.push(`${renderStringify(type)} cannot be used as an entry component.`);
errors.push(`${stringifyForError(type)} cannot be used as an entry component.`);
}
}

View File

@ -11,7 +11,6 @@ import {reflectDependencies} from '../../di/jit/util';
import {Type} from '../../interface/type';
import {Pipe} from '../../metadata/directives';
import {NG_PIPE_DEF} from '../fields';
import {renderStringify} from '../util/misc_utils';
import {angularCoreEnv} from './environment';
@ -20,11 +19,12 @@ export function compilePipe(type: Type<any>, meta: Pipe): void {
Object.defineProperty(type, NG_PIPE_DEF, {
get: () => {
if (ngPipeDef === null) {
ngPipeDef = getCompilerFacade().compilePipe(
angularCoreEnv, `ng://${renderStringify(type)}/ngPipeDef.js`, {
const typeName = type.name;
ngPipeDef =
getCompilerFacade().compilePipe(angularCoreEnv, `ng://${typeName}/ngPipeDef.js`, {
type: type,
typeArgumentCount: 0,
name: type.name,
name: typeName,
deps: reflectDependencies(type),
pipeName: meta.name,
pure: meta.pure !== undefined ? meta.pure : true

View File

@ -15,7 +15,7 @@ import {LContext} from '../interfaces/context';
import {DirectiveDef} from '../interfaces/definition';
import {TElementNode, TNode, TNodeProviderIndexes} from '../interfaces/node';
import {CLEANUP, CONTEXT, FLAGS, HOST, LView, LViewFlags, TVIEW} from '../interfaces/view';
import {renderStringify} from './misc_utils';
import {stringifyForError} from './misc_utils';
import {getLViewParent, getRootContext} from './view_traversal_utils';
import {unwrapRNode} from './view_utils';
@ -191,7 +191,7 @@ export function loadLContext(target: {}, throwOnNotFound: boolean = true): LCont
const context = getLContext(target);
if (!context && throwOnNotFound) {
throw new Error(
ngDevMode ? `Unable to find context associated with ${renderStringify(target)}` :
ngDevMode ? `Unable to find context associated with ${stringifyForError(target)}` :
'Invalid ng target');
}
return context;

View File

@ -6,11 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import {assertDefined} from '../../util/assert';
import {global} from '../../util/global';
import {RElement} from '../interfaces/renderer';
import {CONTEXT, LView, RootContext} from '../interfaces/view';
import {getRootView} from './view_traversal_utils';
/**
* Returns whether the values are different from a change detection stand point.
@ -25,17 +22,31 @@ export function isDifferent(a: any, b: any): boolean {
/**
* Used for stringify render output in Ivy.
* Important! This function is very performance-sensitive and we should
* be extra careful not to introduce megamorphic reads in it.
*/
export function renderStringify(value: any): string {
if (typeof value == 'function') return value.name || value;
if (typeof value == 'string') return value;
if (typeof value === 'function') return value.name || value;
if (typeof value === 'string') return value;
if (value == null) return '';
if (typeof value == 'object' && typeof value.type == 'function')
return value.type.name || value.type;
return '' + value;
}
/**
* Used to stringify a value so that it can be displayed in an error message.
* Important! This function contains a megamorphic read and should only be
* used for error messages.
*/
export function stringifyForError(value: any) {
if (typeof value === 'object' && value != null && typeof value.type === 'function') {
return value.type.name || value.type;
}
return renderStringify(value);
}
export const defaultScheduler =
(typeof requestAnimationFrame !== 'undefined' && requestAnimationFrame || // browser only
setTimeout // everything else

View File

@ -671,6 +671,9 @@
{
"name": "setUpAttributes"
},
{
"name": "stringifyForError"
},
{
"name": "syncViewWithBlueprint"
},

View File

@ -485,6 +485,9 @@
{
"name": "setTNodeAndViewData"
},
{
"name": "stringifyForError"
},
{
"name": "syncViewWithBlueprint"
},

View File

@ -1286,6 +1286,9 @@
{
"name": "stringify"
},
{
"name": "stringifyForError"
},
{
"name": "stylingContext"
},