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.

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

See if trying less inheritance can impact performance #1655

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 7 additions & 14 deletions packages/@glimmer/runtime/lib/vm/attributes/dynamic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function dynamicAttribute(
attr: string,
namespace: Nullable<AttrNamespace>,
isTrusting = false
): DynamicAttribute {
): SimpleDynamicAttribute | DefaultDynamicProperty {
const { tagName, namespaceURI } = element;
const attribute = { element, name: attr, namespace };

Expand All @@ -46,7 +46,7 @@ function buildDynamicAttribute(
tagName: string,
name: string,
attribute: AttributeCursor
): DynamicAttribute {
): SimpleDynamicAttribute | DefaultDynamicProperty {
if (requiresSanitization(tagName, name)) {
return new SafeDynamicAttribute(attribute);
} else {
Expand All @@ -58,7 +58,7 @@ function buildDynamicProperty(
tagName: string,
name: string,
attribute: AttributeCursor
): DynamicAttribute {
): SimpleDynamicAttribute | DefaultDynamicProperty {
if (requiresSanitization(tagName, name)) {
return new SafeDynamicProperty(name, attribute);
}
Expand All @@ -74,14 +74,9 @@ function buildDynamicProperty(
return new DefaultDynamicProperty(name, attribute);
}

export abstract class DynamicAttribute implements AttributeOperation {
export class SimpleDynamicAttribute implements AttributeOperation {
constructor(public attribute: AttributeCursor) {}

abstract set(dom: ElementBuilder, value: unknown, env: Environment): void;
abstract update(value: unknown, env: Environment): void;
}

export class SimpleDynamicAttribute extends DynamicAttribute {
set(dom: ElementBuilder, value: unknown, _env: Environment): void {
const normalizedValue = normalizeValue(value);

Expand All @@ -103,13 +98,11 @@ export class SimpleDynamicAttribute extends DynamicAttribute {
}
}

export class DefaultDynamicProperty extends DynamicAttribute {
export class DefaultDynamicProperty implements AttributeOperation {
constructor(
private normalizedName: string,
attribute: AttributeCursor
) {
super(attribute);
}
public attribute: AttributeCursor
) {}

value: unknown;
set(dom: ElementBuilder, value: unknown, _env: Environment): void {
Expand Down
122 changes: 96 additions & 26 deletions packages/@glimmer/runtime/lib/vm/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import type {
RuntimeContext,
Scope,
SimpleComment,
SimpleElement,
SimpleNode,
UpdatableBlock,
UpdatingOpcode,
UpdatingVM as IUpdatingVM,
Expand Down Expand Up @@ -121,10 +123,20 @@ export class ResumableVMStateImpl implements ResumableVMState {
}
}

export abstract class BlockOpcode implements UpdatingOpcode, Bounds {
export interface BlockOpcode extends UpdatingOpcode, Bounds {
children: UpdatingOpcode[];
bounds: LiveBlock | UpdatableBlock;
parentElement(): SimpleElement;
firstNode(): SimpleNode;
lastNode(): SimpleNode;
evaluate(vm: UpdatingVM): void;
}

export class TryOpcode implements BlockOpcode, ExceptionHandler {
public type = 'try';
public children: UpdatingOpcode[];

protected readonly bounds: LiveBlock;
declare bounds: UpdatableBlock; // Hides property on base class

constructor(
protected state: ResumableVMState,
Expand All @@ -133,7 +145,7 @@ export abstract class BlockOpcode implements UpdatingOpcode, Bounds {
children: UpdatingOpcode[]
) {
this.children = children;
this.bounds = bounds;
this.bounds = bounds as UpdatableBlock;
}

parentElement() {
Expand All @@ -149,16 +161,6 @@ export abstract class BlockOpcode implements UpdatingOpcode, Bounds {
}

evaluate(vm: UpdatingVM) {
vm.try(this.children, null);
}
}

export class TryOpcode extends BlockOpcode implements ExceptionHandler {
public type = 'try';

protected declare bounds: UpdatableBlock; // Hides property on base class

override evaluate(vm: UpdatingVM) {
vm.try(this.children, this);
}

Expand All @@ -183,19 +185,54 @@ export class TryOpcode extends BlockOpcode implements ExceptionHandler {
}
}

export class ListItemOpcode extends TryOpcode {
export class ListItemOpcode implements BlockOpcode, ExceptionHandler {
public retained = false;
public index = -1;
public children: UpdatingOpcode[] = [];

constructor(
state: ResumableVMState,
runtime: RuntimeContext,
bounds: UpdatableBlock,
protected state: ResumableVMState,
protected runtime: RuntimeContext,
public bounds: UpdatableBlock,
public key: unknown,
public memo: Reference,
public value: Reference
) {
super(state, runtime, bounds, []);
) {}

parentElement() {
return this.bounds.parentElement();
}

firstNode() {
return this.bounds.firstNode();
}

lastNode() {
return this.bounds.lastNode();
}

evaluate(vm: UpdatingVM) {
vm.try(this.children, this);
}

handleException() {
let { state, bounds, runtime } = this;

destroyChildren(this);

let elementStack = NewElementBuilder.resume(runtime.env, bounds);
let vm = state.resume(runtime, elementStack);

let updating: UpdatingOpcode[] = [];
let children = (this.children = []);

let result = vm.execute((vm) => {
vm.pushUpdating(updating);
vm.updateWith(this);
vm.pushUpdating(children);
});

associateDestroyableChild(this, result.drop);
}

updateReferences(item: OpaqueIterationItem) {
Expand All @@ -213,33 +250,46 @@ export class ListItemOpcode extends TryOpcode {
}
}

export class ListBlockOpcode extends BlockOpcode {
export class ListBlockOpcode implements BlockOpcode, ExceptionHandler {
public type = 'list-block';
public declare children: ListItemOpcode[];

private opcodeMap = new Map<unknown, ListItemOpcode>();
private marker: SimpleComment | null = null;
private lastIterator: OpaqueIterator;

protected declare readonly bounds: LiveBlockList;
bounds: LiveBlockList;

constructor(
state: ResumableVMState,
runtime: RuntimeContext,
protected state: ResumableVMState,
protected runtime: RuntimeContext,
bounds: LiveBlockList,
children: ListItemOpcode[],
private iterableRef: Reference<OpaqueIterator>
) {
super(state, runtime, bounds, children);
this.children = children;
this.bounds = bounds;
this.lastIterator = valueForRef(iterableRef);
}

parentElement() {
return this.bounds.parentElement();
}

firstNode() {
return this.bounds.firstNode();
}

lastNode() {
return this.bounds.lastNode();
}

initializeChild(opcode: ListItemOpcode) {
opcode.index = this.children.length - 1;
this.opcodeMap.set(opcode.key, opcode);
}

override evaluate(vm: UpdatingVM) {
evaluate(vm: UpdatingVM) {
let iterator = valueForRef(this.iterableRef);

if (this.lastIterator !== iterator) {
Expand All @@ -261,7 +311,27 @@ export class ListBlockOpcode extends BlockOpcode {
}

// Run now-updated updating opcodes
super.evaluate(vm);
vm.try(this.children, this);
}

handleException() {
let { state, bounds, runtime } = this;

destroyChildren(this);

let elementStack = NewElementBuilder.resume(runtime.env, bounds);
let vm = state.resume(runtime, elementStack);

let updating: UpdatingOpcode[] = [];
let children = (this.children = []);

let result = vm.execute((vm) => {
vm.pushUpdating(updating);
vm.updateWith(this);
vm.pushUpdating(children);
});

associateDestroyableChild(this, result.drop);
}

private sync(iterator: OpaqueIterator) {
Expand Down
Loading