Skip to content

Commit 670fb6b

Browse files
authored
compiler: implement pos-only mode for implicit spaces (#602)
Implement a "pos-only" mode for implicit space skipping. In pos-only mode, we don't build a CST, do failure recording, etc. This results in a significant performance improvement. **Before** ``` $ bin/es5bench-wasm Compile: 66ms ...... JS match: 3312.9ms ± 284.9ms (n=3) Wasm match: 79.1ms ± 26.7ms (n=3) Wasm vs JS match: 41.9x Wasm memory: 174.0MB ``` **After** ``` $ bin/es5bench-wasm Compile: 57ms ...... JS match: 3311.9ms ± 113.2ms (n=3) Wasm match: 55.8ms ± 15.7ms (n=3) Wasm vs JS match: 59.3x Wasm memory: 87.0MB ```
1 parent 35e4185 commit 670fb6b

4 files changed

Lines changed: 133 additions & 23 deletions

File tree

packages/compiler/runtime/ohmRuntime.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,8 @@ function useMemoizedResult(ruleId: i32, result: MemoEntry): ApplyResult {
299299

300300
// Evaluate $spaces without allocating a CST node or pushing a binding.
301301
// Stores a sentinel in the memo table encoding just the match length.
302+
// Calls the pos-only compiled version of $spaces (at table index numRules + 1),
303+
// which skips all CST building internally.
302304
export function evalSpacesImplicit(): void {
303305
const memo = memoTableGet(pos, IMPLICIT_SPACES_RULE_ID);
304306
if (memo !== EMPTY) {
@@ -307,11 +309,8 @@ export function evalSpacesImplicit(): void {
307309
return;
308310
}
309311
const origPos = pos;
310-
const origChunk = bindingsChunk;
311-
const origIdx = bindingsIdx;
312-
evalRuleBody(IMPLICIT_SPACES_RULE_ID);
312+
call_indirect<RuleEvalResult>(numRules + 1);
313313
const matchLen = <i32>pos - <i32>origPos;
314-
restoreBindings(origChunk, origIdx); // discard child bindings
315314
memoTableSet(origPos, IMPLICIT_SPACES_RULE_ID, (matchLen << 2) | MEMO_SPACES_FLAG);
316315
}
317316

packages/compiler/src/Compiler.ts

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ class Assembler {
254254
_frameDepth: number;
255255
_savedAtDepthStack: Set<string>[];
256256
useChunkedBindings: boolean;
257+
posOnlyMode: boolean;
257258

258259
// In WebAssembly, function indices (funcidx) are a flat numbering: imports
259260
// come first, then locally-defined functions. The prebuilt module has its
@@ -281,6 +282,7 @@ class Assembler {
281282
this._frameDepth = 0;
282283
this._savedAtDepthStack = [];
283284
this.useChunkedBindings = useChunkedBindings;
285+
this.posOnlyMode = false;
284286

285287
this._typeMap = typeMap;
286288
}
@@ -627,6 +629,9 @@ class Assembler {
627629
}
628630

629631
saveNumBindings(): SavedBindingsState {
632+
if (this.posOnlyMode) {
633+
return noopBacktrackPoint.bindings;
634+
}
630635
if (!this.useChunkedBindings) {
631636
// Array mode: save a single length value.
632637
this.callPrebuiltFunc('getBindingsLength');
@@ -707,6 +712,7 @@ class Assembler {
707712
}
708713

709714
maybeRecordFailure(origPosThunk: () => void, failureId: number): void {
715+
if (this.posOnlyMode) return;
710716
this.globalGet('errorMessagePos');
711717
this.i32Const(0);
712718
this.emit(instr.i32.ge_s);
@@ -810,6 +816,10 @@ class Assembler {
810816
}
811817

812818
newIterNode(saved: SavedBacktrackPoint, arity: number, isOpt = false): void {
819+
if (this.posOnlyMode) {
820+
this.i32Const(1);
821+
return;
822+
}
813823
saved.pos.get();
814824
this.globalGet('pos');
815825
saved.bindings.getChunk();
@@ -822,6 +832,10 @@ class Assembler {
822832
// Wrap the bindings accumulated since the last pushDepth() in a
823833
// nonterminal node.
824834
newNonterminalNode(saved: SavedBacktrackPoint, ruleId: number): void {
835+
if (this.posOnlyMode) {
836+
this.i32Const(1);
837+
return;
838+
}
825839
saved.pos.get();
826840
this.globalGet('pos');
827841
this.i32Const(ruleId);
@@ -833,6 +847,10 @@ class Assembler {
833847

834848
// [startIdx: i32] -> [tagged: i32]
835849
newTerminalNode(): void {
850+
if (this.posOnlyMode) {
851+
this.i32Const(1);
852+
return;
853+
}
836854
this.localGet('postSpacesPos');
837855
this.globalGet('pos');
838856
this.callPrebuiltFunc('newTerminalNode');
@@ -854,17 +872,20 @@ class Assembler {
854872
}
855873

856874
pushFluffySavePoint(): void {
875+
if (this.posOnlyMode) return;
857876
this.callPrebuiltFunc('pushFluffySavePoint');
858877
}
859878

860879
popFluffySavePoint(shouldMark: boolean): void {
880+
if (this.posOnlyMode) return;
861881
this.i32Const(shouldMark ? 1 : 0);
862882
this.callPrebuiltFunc('popFluffySavePoint');
863883
}
864884

865885
// Pop the fluffy save point, marking failures as fluffy only when
866886
// pos matches errorMessagePos. This mirrors ohm-js's scoped failure recording.
867887
popFluffySavePointIfAtErrorPos(): void {
888+
if (this.posOnlyMode) return;
868889
this.globalGet('errorMessagePos');
869890
this.globalGet('pos');
870891
this.i32Eq();
@@ -875,6 +896,7 @@ class Assembler {
875896
// On success, mark failures as fluffy if pos is at errorMessagePos;
876897
// on failure, discard without marking.
877898
popFluffySavePointOnResult(): void {
899+
if (this.posOnlyMode) return;
878900
this.localGet('ret');
879901
this.ifElse(
880902
w.blocktype.empty,
@@ -1262,10 +1284,11 @@ export class Compiler {
12621284
const restoreFailurePos = name === '$spaces';
12631285

12641286
const descriptionId = ruleInfo.description ? this._strings.add(ruleInfo.description) : -1;
1265-
const hasDescription = descriptionId >= 0;
1287+
const hasDescription = !asm.posOnlyMode && descriptionId >= 0;
12661288

1289+
const funcName = asm.posOnlyMode ? `$${name}_posOnly` : `$${name}`;
12671290
this.beginLexContext(!ruleInfo.isSyntactic);
1268-
asm.addFunction(`$${name}`, paramTypes, [w.valtype.i32], () => {
1291+
asm.addFunction(funcName, paramTypes, [w.valtype.i32], () => {
12691292
asm.addLocal('ret', w.valtype.i32);
12701293
asm.addLocal('tmp', w.valtype.i32);
12711294
asm.addLocal('postSpacesPos', w.valtype.i32);
@@ -1605,10 +1628,11 @@ export class Compiler {
16051628
exports.push(w.export_(name, [0x03, prebuilt.globalidxByName[name]]));
16061629
}
16071630
// The module will have a table containing references to all of the rule eval functions,
1608-
// plus a compiler-generated $isRuleSyntactic dispatch function at the end.
1609-
// The rule ID can be used directly as the table index; $isRuleSyntactic is at index numRules.
1631+
// plus compiler-generated helper functions at the end:
1632+
// [numRules] = $isRuleSyntactic
1633+
// [numRules + 1] = $spaces pos-only (for evalSpacesImplicit)
16101634
const numRules = this.ruleIdByName.size;
1611-
const tableSize = numRules + 1; // +1 for $isRuleSyntactic
1635+
const tableSize = numRules + 2; // +1 for $isRuleSyntactic, +1 for $spaces pos-only
16121636
const table = w.table(
16131637
w.tabletype(w.elemtype.funcref, w.limits.minmax(tableSize, tableSize))
16141638
);
@@ -1617,6 +1641,10 @@ export class Compiler {
16171641
const isRuleSyntacticIdx = functionDecls.findIndex(f => f.name === '$isRuleSyntactic');
16181642
assert(isRuleSyntacticIdx !== -1, 'No $isRuleSyntactic function found');
16191643
tableData.push(w.funcidx(compilerFuncOffset + isRuleSyntacticIdx));
1644+
// Add $spaces pos-only as the next table entry.
1645+
const spacesPosOnlyIdx = functionDecls.findIndex(f => f.name === '$$spaces_posOnly');
1646+
assert(spacesPosOnlyIdx !== -1, 'No $$spaces_posOnly function found');
1647+
tableData.push(w.funcidx(compilerFuncOffset + spacesPosOnlyIdx));
16201648
assert(tableSize === tableData.length, 'Invalid table size');
16211649

16221650
// Determine the index of the start function.
@@ -1772,6 +1800,12 @@ export class Compiler {
17721800
});
17731801
ruleDecls.push(checkNotNull(asm._functionDecls.at(-1)));
17741802

1803+
// Compile a pos-only version of $spaces that skips all CST building.
1804+
// This goes into the table at index numRules + 1.
1805+
asm.posOnlyMode = true;
1806+
ruleDecls.push(this.compileRule('$spaces'));
1807+
asm.posOnlyMode = false;
1808+
17751809
return ruleDecls;
17761810
}
17771811

@@ -2009,7 +2043,7 @@ export class Compiler {
20092043

20102044
const {asm} = this;
20112045

2012-
if (this.shouldInlineRule(exp.ruleName)) {
2046+
if (this.shouldInlineRule(exp.ruleName) || asm.posOnlyMode) {
20132047
this.emitInlinedApply(exp);
20142048
return;
20152049
}

packages/compiler/test/test-wasm.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,48 @@ test('cst: leadingSpaces children via lazy parsing', async t => {
212212
});
213213
});
214214

215+
test('cst: leadingSpaces with custom spaces rule', async t => {
216+
const g = await compileAndLoad(`G {
217+
Start = word+
218+
word = letter+
219+
space += comment
220+
comment = "//" (~"\\n" any)*
221+
}`);
222+
g.match('abc // yo\n def').use(r => {
223+
t.true(r.succeeded());
224+
const root = r.getCstRoot();
225+
226+
// The Plus list should contain two words.
227+
const list = root.children[0];
228+
t.is(list.children.length, 2);
229+
230+
const [word1, word2] = list.children;
231+
t.is(word1.sourceString, 'abc');
232+
t.is(word2.sourceString, 'def');
233+
234+
// word2 should have leadingSpaces covering the comment and whitespace.
235+
const spaces = word2.leadingSpaces;
236+
t.truthy(spaces);
237+
t.is(spaces.ctorName, 'spaces');
238+
t.is(spaces.sourceString, ' // yo\n ');
239+
t.is(spaces.matchLength, 8);
240+
241+
// Lazy CST children should have correct source strings.
242+
const starList = spaces.children[0];
243+
t.true(starList.isList());
244+
t.is(starList.children.length, 4);
245+
t.is(starList.children[0].sourceString, ' ');
246+
t.is(starList.children[1].sourceString, '// yo');
247+
t.is(starList.children[2].sourceString, '\n');
248+
t.is(starList.children[3].sourceString, ' ');
249+
250+
// The comment child should be a 'space' wrapping a 'comment'.
251+
const commentSpace = starList.children[1];
252+
t.is(commentSpace.ctorName, 'space');
253+
t.is(commentSpace.children[0].ctorName, 'comment');
254+
});
255+
});
256+
215257
test('cst: lazy parsing survives memory.grow()', async t => {
216258
const g = await compileAndLoad('G { Start = "x" }');
217259
g.match(' x').use(r => {

packages/runtime/src/miniohm.ts

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,20 @@ const UnicodeCategoryNames = [
5454
const utf8 = new TextDecoder('utf-8');
5555
const utf16le = new TextDecoder('utf-16le');
5656

57+
function isSyntacticRuleName(ruleName: string): boolean {
58+
const firstChar = ruleName[0];
59+
return firstChar === firstChar.toUpperCase();
60+
}
61+
62+
function getRuleId(ctx: MatchContext, ptr: number): number {
63+
return ctx.view.getInt32(ptr + CST_TYPE_AND_DETAILS_OFFSET, true) >>> 2;
64+
}
65+
66+
// Check whether the nonterminal at `ptr` is a syntactic rule (name starts with uppercase).
67+
function isSyntacticRule(ctx: MatchContext, ptr: number): boolean {
68+
return ctx.ruleIsSyntactic[getRuleId(ctx, ptr)];
69+
}
70+
5771
// Minimal implementation of Interval (for FailedMatchResult)
5872
export class Interval {
5973
startIdx: number;
@@ -199,6 +213,8 @@ export class Grammar {
199213
/** @internal */
200214
private _ruleNames: string[] = [];
201215
/** @internal */
216+
private _ruleIsSyntactic: boolean[] = [];
217+
/** @internal */
202218
private _input = '';
203219

204220
/** @internal */
@@ -319,9 +335,11 @@ export class Grammar {
319335
private _extractStrings(module: WebAssembly.Module): void {
320336
assert(this._ruleNames.length === 0);
321337
assert(this._ruleIds.size === 0);
338+
assert(this._ruleIsSyntactic.length === 0);
322339
for (const ruleName of parseStringTable(module, 'ruleNames')) {
323340
this._ruleIds.set(ruleName, this._ruleIds.size);
324341
this._ruleNames.push(ruleName);
342+
this._ruleIsSyntactic.push(isSyntacticRuleName(ruleName));
325343
}
326344
for (const str of parseStringTable(module, 'strings')) {
327345
this._strings.push(str);
@@ -363,6 +381,7 @@ export class Grammar {
363381

364382
const ctx: MatchContext = {
365383
ruleNames: this._ruleNames,
384+
ruleIsSyntactic: this._ruleIsSyntactic,
366385
view: new DataView(buffer),
367386
input,
368387
getSpacesLenAt: exports.getSpacesLenAt,
@@ -417,6 +436,7 @@ export class Grammar {
417436
const {exports} = this._instance as any;
418437
ctx ??= {
419438
ruleNames: this._ruleNames,
439+
ruleIsSyntactic: this._ruleIsSyntactic,
420440
view: new DataView(exports.memory.buffer),
421441
input: this._input,
422442
getSpacesLenAt: exports.getSpacesLenAt,
@@ -425,7 +445,7 @@ export class Grammar {
425445
};
426446
const spacesLen = Math.max(0, exports.getSpacesLenAt(0));
427447
const rootAddr = exports.bindingsAt(0);
428-
const root = new CstNodeImpl(ctx, rootAddr, spacesLen);
448+
const root = new CstNodeImpl(ctx, rootAddr, spacesLen, isSyntacticRule(ctx, rootAddr));
429449
if (spacesLen > 0) {
430450
root.leadingSpaces = new LazySpacesNode(ctx, 0, spacesLen);
431451
}
@@ -449,6 +469,7 @@ export class Grammar {
449469

450470
export interface MatchContext {
451471
ruleNames: string[];
472+
ruleIsSyntactic: boolean[];
452473
view: DataView;
453474
input: string;
454475
getSpacesLenAt?: (pos: number) => number;
@@ -532,11 +553,17 @@ class CstNodeImpl implements CstNodeBase {
532553
leadingSpaces?: NonterminalNode = undefined;
533554
source: {startIdx: number; endIdx: number};
534555

535-
constructor(ctx: MatchContext, ptr: number, startIdx: number) {
556+
// Whether this node's children are in a syntactic context (i.e., have
557+
// implicit space skipping between them). Nonterminals set this from
558+
// their rule id; other node types inherit from their parent.
559+
_syntactic!: boolean;
560+
561+
constructor(ctx: MatchContext, ptr: number, startIdx: number, syntactic?: boolean) {
536562
// Non-enumerable properties
537563
Object.defineProperties(this, {
538564
_ctx: {value: ctx},
539565
_children: {writable: true},
566+
_syntactic: {value: syntactic ?? false, writable: true},
540567
});
541568
this._base = ptr;
542569
this.startIdx = startIdx;
@@ -663,15 +690,17 @@ class CstNodeImpl implements CstNodeBase {
663690
const children: (CstNodeImpl | TaggedTerminalNode)[] = [];
664691
let {startIdx} = this;
665692
const {getSpacesLenAt} = this._ctx;
693+
// Only look up implicit spaces when we're in a syntactic context.
694+
const doSpaceLookup = this._syntactic && !!getSpacesLenAt;
666695
for (let i = 0; i < this.count; i++) {
667696
const slotOffset = this._base + CST_CHILDREN_OFFSET + i * 4;
668697
const ptr = this._ctx.view.getUint32(slotOffset, true);
669698

670699
if (isTaggedTerminal(ptr)) {
671700
// Tagged terminals always have parent-level space skipping.
672701
let spacesLen = 0;
673-
if (getSpacesLenAt) {
674-
spacesLen = Math.max(0, getSpacesLenAt(startIdx));
702+
if (doSpaceLookup) {
703+
spacesLen = Math.max(0, getSpacesLenAt!(startIdx));
675704
if (spacesLen > 0) startIdx += spacesLen;
676705
}
677706
const node = new TaggedTerminalNode(this._ctx, ptr, startIdx);
@@ -686,18 +715,24 @@ class CstNodeImpl implements CstNodeBase {
686715
// Only query spaces for terminals and nonterminals — not for
687716
// iteration (ITER_FLAG) or optional nodes, which handle space
688717
// skipping internally.
689-
const type = (this._ctx.view.getInt32(ptr + CST_TYPE_AND_DETAILS_OFFSET, true) &
690-
MATCH_RECORD_TYPE_MASK) as MatchRecordType;
718+
const typeAndDetails = this._ctx.view.getInt32(ptr + CST_TYPE_AND_DETAILS_OFFSET, true);
719+
const type = (typeAndDetails & MATCH_RECORD_TYPE_MASK) as MatchRecordType;
691720
let spacesLen = 0;
692721
if (
693-
getSpacesLenAt &&
722+
doSpaceLookup &&
694723
(type === MatchRecordType.NONTERMINAL || type === MatchRecordType.TERMINAL)
695724
) {
696-
spacesLen = Math.max(0, getSpacesLenAt(startIdx));
725+
spacesLen = Math.max(0, getSpacesLenAt!(startIdx));
697726
if (spacesLen > 0) startIdx += spacesLen;
698727
}
699728

700-
const node = new CstNodeImpl(this._ctx, ptr, startIdx);
729+
// Nonterminals determine syntactic context from their name;
730+
// other node types (iter, opt) inherit from the parent.
731+
const childSyntactic =
732+
type === MatchRecordType.NONTERMINAL
733+
? this._ctx.ruleIsSyntactic[typeAndDetails >>> 2]
734+
: this._syntactic;
735+
const node = new CstNodeImpl(this._ctx, ptr, startIdx, childSyntactic);
701736
if (spacesLen > 0) {
702737
node.leadingSpaces = new LazySpacesNode(this._ctx, startIdx - spacesLen, spacesLen);
703738
}
@@ -713,13 +748,12 @@ class CstNodeImpl implements CstNodeBase {
713748

714749
isSyntactic(): boolean {
715750
assert(this.isNonterminal(), 'Not a nonterminal');
716-
const firstChar = this.ctorName[0];
717-
return firstChar === firstChar.toUpperCase();
751+
return this._syntactic;
718752
}
719753

720754
isLexical(): boolean {
721755
assert(this.isNonterminal(), 'Not a nonterminal');
722-
return !this.isSyntactic();
756+
return !this._syntactic;
723757
}
724758

725759
toString(): string {
@@ -824,7 +858,8 @@ class LazySpacesNode implements NonterminalNode {
824858
if (memory && this._ctx.view.buffer !== memory.buffer) {
825859
this._ctx.view = new DataView(memory.buffer);
826860
}
827-
const fullNode = new CstNodeImpl(this._ctx, ptr, this._startIdx);
861+
// The spaces rule is lexical, so pass syntactic=false.
862+
const fullNode = new CstNodeImpl(this._ctx, ptr, this._startIdx, false);
828863
return fullNode.children;
829864
}
830865

0 commit comments

Comments
 (0)