Skip to content

Commit

Permalink
WIP fix template size regression
Browse files Browse the repository at this point in the history
  • Loading branch information
@krisselden
krisselden committed Mar 11, 2020
1 parent c9756de commit f87c999
Show file tree
Hide file tree
Showing 11 changed files with 288 additions and 161 deletions.
11 changes: 3 additions & 8 deletions packages/@glimmer/compiler/lib/builder-interface.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Dict, Option, ExpressionContext } from '@glimmer/interfaces';
import { Dict, Option } from '@glimmer/interfaces';
import { dict, assertNever, expect } from '@glimmer/util';

export type BuilderParams = BuilderExpression[];
Expand Down Expand Up @@ -106,7 +106,7 @@ export type NormalizedStatement =
export function normalizeStatement(statement: BuilderStatement): NormalizedStatement {
if (Array.isArray(statement)) {
if (statementIsExpression(statement)) {
return normalizeAppendExpression(statement, ExpressionContext.AppendSingleId);
return normalizeAppendExpression(statement);
} else if (isSugaryArrayStatement(statement)) {
return normalizeSugaryArrayStatement(statement);
} else {
Expand Down Expand Up @@ -223,11 +223,7 @@ function normalizeVerboseStatement(statement: VerboseStatement): NormalizedState
}

case Builder.Append: {
return normalizeAppendExpression(
statement[1],
ExpressionContext.AppendSingleId,
statement[2]
);
return normalizeAppendExpression(statement[1], statement[2]);
}

case Builder.Modifier: {
Expand Down Expand Up @@ -588,7 +584,6 @@ export type NormalizedExpression =

export function normalizeAppendExpression(
expression: BuilderExpression,
_context: ExpressionContext,
forceTrusted = false
): AppendExpr | AppendPath {
if (expression === null || expression === undefined) {
Expand Down
81 changes: 65 additions & 16 deletions packages/@glimmer/compiler/lib/builder.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { WireFormat, Option, Dict, Expressions, ExpressionContext } from '@glimmer/interfaces';

import Op = WireFormat.SexpOpcodes;
import { dict, assertNever, assert, values } from '@glimmer/util';
import { dict, assertNever, assert, values, exhausted } from '@glimmer/util';
import {
BuilderStatement,
BuilderComment,
Expand Down Expand Up @@ -192,8 +192,6 @@ export function buildStatement(
[
Op.Append,
+normalized.trusted,
0,
0,
buildPath(normalized.path, ExpressionContext.AppendSingleId, symbols),
],
];
Expand All @@ -204,8 +202,6 @@ export function buildStatement(
[
Op.Append,
+normalized.trusted,
0,
0,
buildExpression(normalized.expr, ExpressionContext.Expression, symbols),
],
];
Expand All @@ -217,11 +213,11 @@ export function buildStatement(
let builtHash: WireFormat.Core.Hash = hash ? buildHash(hash, symbols) : null;
let builtExpr: WireFormat.Expression = buildPath(path, ExpressionContext.CallHead, symbols);

return [[Op.Append, +trusted, 0, 0, [Op.Call, 0, 0, builtExpr, builtParams, builtHash]]];
return [[Op.Append, +trusted, [Op.Call, builtExpr, builtParams, builtHash]]];
}

case HeadKind.Literal: {
return [[Op.Append, 1, 0, 0, normalized.value]];
return [[Op.Append, 1, normalized.value]];
}

case HeadKind.Comment: {
Expand Down Expand Up @@ -443,7 +439,7 @@ export function buildExpression(
let builtHash = buildHash(expr.hash, symbols);
let builtExpr = buildPath(expr.path, ExpressionContext.CallHead, symbols);

return [Op.Call, 0, 0, builtExpr, builtParams, builtHash];
return [Op.Call, builtExpr, builtParams, builtHash];
}

case ExpressionKind.HasBlock: {
Expand Down Expand Up @@ -477,34 +473,87 @@ export function buildExpression(
}
}
}

export function buildPath(
path: Path,
context: ExpressionContext,
symbols: Symbols
): Expressions.GetPath {
if (path.tail.length === 0) {
return [Op.GetPath, buildVar(path.variable, context, symbols), path.tail];
return buildVar(path.variable, context, symbols, path.tail);
} else {
return [Op.GetPath, buildVar(path.variable, ExpressionContext.Expression, symbols), path.tail];
return buildVar(path.variable, ExpressionContext.Expression, symbols, path.tail);
}
}

export function buildVar(
head: Variable,
context: ExpressionContext,
symbols: Symbols,
path: string[]
): Expressions.GetPath;
export function buildVar(
head: Variable,
context: ExpressionContext,
symbols: Symbols
): Expressions.GetSymbol | Expressions.GetFree | Expressions.GetContextualFree {
): Expressions.Get;
export function buildVar(
head: Variable,
context: ExpressionContext,
symbols: Symbols,
path?: string[]
): Expressions.GetPath | Expressions.Get {
let op: Expressions.Get[0] = Op.GetSymbol;
let sym: number;
switch (head.kind) {
case VariableKind.Free:
return [Op.GetContextualFree, symbols.freeVar(head.name), context];
op = expressionContextOp(context);
sym = symbols.freeVar(head.name);
break;
default:
op = Op.GetSymbol;
sym = getSymbolForVar(head.kind, symbols, head.name);
}
return (path === undefined ? [op, sym] : [op, sym, path]) as
| Expressions.Get
| Expressions.GetPath;
}

function getSymbolForVar(
kind: Exclude<VariableKind, VariableKind.Free>,
symbols: Symbols,
name: string
) {
switch (kind) {
case VariableKind.Arg:
return [Op.GetSymbol, symbols.arg(head.name)];
return symbols.arg(name);
case VariableKind.Block:
return [Op.GetSymbol, symbols.block(head.name)];
return symbols.block(name);
case VariableKind.Local:
return [Op.GetSymbol, symbols.local(head.name)];
return symbols.local(name);
case VariableKind.This:
return [Op.GetSymbol, symbols.this()];
return symbols.this();
default:
return exhausted(kind);
}
}

export function expressionContextOp(context: ExpressionContext) {
switch (context) {
case ExpressionContext.AppendSingleId:
return Op.GetFreeInAppendSingleId;
case ExpressionContext.Expression:
return Op.GetFreeInExpression;
case ExpressionContext.CallHead:
return Op.GetFreeInCallHead;
case ExpressionContext.BlockHead:
return Op.GetFreeInBlockHead;
case ExpressionContext.ModifierHead:
return Op.GetFreeInModifierHead;
case ExpressionContext.ComponentHead:
return Op.GetFreeInComponentHead;
default:
return exhausted(context);
}
}

Expand Down
2 changes: 0 additions & 2 deletions packages/@glimmer/compiler/lib/compiler-ops.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ export interface InputOps {
comment: [AST.CommentStatement];
}

// type Location = ['loc', [null, [number, number], [number, number]]];

export interface AllocateSymbolsOps {
startProgram: AST.Template;
endProgram: void;
Expand Down
42 changes: 10 additions & 32 deletions packages/@glimmer/compiler/lib/javascript-compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Expressions,
ExpressionContext,
} from '@glimmer/interfaces';
import { expressionContextOp } from './builder';

export type str = string;
import Core = WireFormat.Core;
Expand Down Expand Up @@ -236,11 +237,11 @@ export default class JavaScriptCompiler implements Processor<JavaScriptCompilerO
/// Statements

text(content: string) {
this.push([SexpOpcodes.Append, 1, 0, 0, content]);
this.push([SexpOpcodes.Append, 1, content]);
}

append(trusted: boolean) {
this.push([SexpOpcodes.Append, +trusted, 0, 0, this.popValue<Expression>()]);
this.push([SexpOpcodes.Append, +trusted, this.popValue<Expression>()]);
}

comment(value: string) {
Expand All @@ -251,7 +252,7 @@ export default class JavaScriptCompiler implements Processor<JavaScriptCompilerO
let name = this.popValue<Expression>();
let params = this.popValue<Params>();
let hash = this.popValue<Hash>();
this.push([SexpOpcodes.Modifier, 0, 0, name, params, hash]);
this.push([SexpOpcodes.Modifier, name, params, hash]);
}

block([template, inverse]: [number, Option<number>]) {
Expand Down Expand Up @@ -424,9 +425,9 @@ export default class JavaScriptCompiler implements Processor<JavaScriptCompilerO
}
}

getPath(rest: string[]) {
let head = this.popValue<Expressions.Get>();
this.pushValue<Expressions.GetPath>([SexpOpcodes.GetPath, head, rest]);
getPath(path: string[]) {
let [op, sym] = this.popValue<Expressions.Get>();
this.pushValue<Expressions.GetPath>([op, sym, path]);
}

getSymbol(head: number) {
Expand All @@ -438,26 +439,19 @@ export default class JavaScriptCompiler implements Processor<JavaScriptCompilerO
}

getFreeWithContext([head, context]: [number, ExpressionContext]) {
this.pushValue<Expressions.GetContextualFree>([SexpOpcodes.GetContextualFree, head, context]);
this.pushValue<Expressions.GetContextualFree>([expressionContextOp(context), head]);
}

concat() {
this.pushValue<Expressions.Concat>([SexpOpcodes.Concat, this.popValue<ConcatParams>()]);
}

helper() {
let { value: head, location } = this.popLocatedValue<Expression>();
let { value: head } = this.popLocatedValue<Expression>();
let params = this.popValue<Params>();
let hash = this.popValue<Hash>();

this.pushValue<Expressions.Helper>([
SexpOpcodes.Call,
start(location),
end(location),
head,
params,
hash,
]);
this.pushValue<Expressions.Helper>([SexpOpcodes.Call, head, params, hash]);
}

/// Stack Management Opcodes
Expand Down Expand Up @@ -537,19 +531,3 @@ export default class JavaScriptCompiler implements Processor<JavaScriptCompilerO
return this.popLocatedValue<T>().value;
}
}

function start(location: Option<SourceLocation>): number {
if (location) {
return location.start;
} else {
return -1;
}
}

function end(location: Option<SourceLocation>): number {
if (location) {
return location.end - location.start;
} else {
return -1;
}
}
21 changes: 13 additions & 8 deletions packages/@glimmer/compiler/lib/template-compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,17 +261,23 @@ export default class TemplateCompiler implements Processor<InputOps> {

private argPath(head: string, rest: string[], loc: AST.BaseNode) {
this.opcode(['getArg', head], loc);
this.opcode(['getPath', rest], loc);
if (rest.length > 0) {
this.opcode(['getPath', rest], loc);
}
}

private varPath(head: string, rest: string[], context: ExpressionContext, loc: AST.BaseNode) {
this.opcode(['getVar', [head, context]], loc);
this.opcode(['getPath', rest], loc);
if (rest.length > 0) {
this.opcode(['getPath', rest], loc);
}
}

private thisPath(rest: string[], loc: AST.BaseNode) {
this.opcode(['getThis'], loc);
this.opcode(['getPath', rest], loc);
if (rest.length > 0) {
this.opcode(['getPath', rest], loc);
}
}

private expression(path: AST.Expression, context: ExpressionContext, expr: AST.Node) {
Expand Down Expand Up @@ -339,14 +345,13 @@ export default class TemplateCompiler implements Processor<InputOps> {
}

private path(expr: AST.PathExpression, context: ExpressionContext) {
let [head, ...rest] = expr.parts;

let { parts } = expr;
if (expr.data) {
this.argPath(`@${head}`, rest, expr);
this.argPath(`@${parts[0]}`, parts.slice(1), expr);
} else if (expr.this) {
this.thisPath(expr.parts, expr);
this.thisPath(parts, expr);
} else {
this.varPath(head, rest, context, expr);
this.varPath(parts[0], parts.slice(1), context, expr);
}
}

Expand Down
Loading

0 comments on commit f87c999

Please sign in to comment.