Skip to content

Commit

Permalink
perf(ivy): avoid memory allocation in the isAnimationProp check (angu…
Browse files Browse the repository at this point in the history
…lar#32997)

Accessing a string's character at index allocates a new, single character string.
A better (faster) check is to use `charCodeAt` that doesn't trigger allocation.

This simple change speeds up the element_text_create benchmark by ~7%.

PR Close angular#32997
  • Loading branch information
pkozlowski-opensource authored and ODAVING committed Oct 18, 2019
1 parent 7407e0b commit 734bff0
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 12 deletions.
5 changes: 2 additions & 3 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {isNodeMatchingSelectorList} from '../node_selector_matcher';
import {ActiveElementFlags, executeElementExitFn, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, hasActiveElementFlag, incrementActiveDirectiveId, namespaceHTMLInternal, selectView, setActiveHostElement, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
import {renderStylingMap} from '../styling/bindings';
import {NO_CHANGE} from '../tokens';
import {ANIMATION_PROP_PREFIX, isAnimationProp} from '../util/attrs_utils';
import {isAnimationProp} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils';
import {getLViewParent} from '../util/view_traversal_utils';
import {getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';
Expand Down Expand Up @@ -980,8 +980,7 @@ function validateProperty(
// The property is considered valid if the element matches the schema, it exists on the element
// or it is synthetic, and we are in a browser context (web worker nodes should be skipped).
return matchingSchemas(hostView, tNode.tagName) || propName in element ||
propName[0] === ANIMATION_PROP_PREFIX || typeof Node !== 'function' ||
!(element instanceof Node);
isAnimationProp(propName) || typeof Node !== 'function' || !(element instanceof Node);
}

function matchingSchemas(hostView: LView, tagName: string | null): boolean {
Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/render3/util/attrs_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ export function isNameOnlyAttributeMarker(marker: string | AttributeMarker | Css
marker === AttributeMarker.I18n;
}

export const ANIMATION_PROP_PREFIX = '@';

export function isAnimationProp(name: string): boolean {
return name[0] === ANIMATION_PROP_PREFIX;
// Perf note: accessing charCodeAt to check for the first character of a string is faster as
// compared to accessing a character at index 0 (ex. name[0]). The main reason for this is that
// charCodeAt doesn't allocate memory to return a substring.
return name.charCodeAt(0) === 64; // @
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
{
"name": "ACTIVE_INDEX"
},
{
"name": "ANIMATION_PROP_PREFIX"
},
{
"name": "BINDING_INDEX"
},
Expand Down
3 changes: 0 additions & 3 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
{
"name": "ACTIVE_INDEX"
},
{
"name": "ANIMATION_PROP_PREFIX"
},
{
"name": "BINDING_INDEX"
},
Expand Down

0 comments on commit 734bff0

Please sign in to comment.