From cb71fc2f6a4d64ff220f55dbf9116aa5dc77539f Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 27 Jun 2026 18:57:38 +0100 Subject: [PATCH 1/2] perf(engine): make for-in key dedup O(k) with a hash set for...in enumeration deduplicated keys across the prototype chain with TStringList.IndexOf on an unsorted list, an O(k) linear scan run once per key, so a full enumeration was O(k^2) in the total (own + inherited) key count. Replace the TStringList with a TOrderedStringMap hash set (ContainsKey + Add), the established string-set idiom in the codebase, in both execution engines: CreateForInEntriesArray (tree-walking interpreter) and ForInEntriesArray (bytecode VM). Membership stays case-sensitive (native string equality) and per-level enumeration order is unchanged (OrderForInPropertyKeys / VMOrderOwnPropertyStringKeys still own ordering; the set is membership-only), so behavior is identical. Add a regression test for same-name enumerable shadowing and dedup across a deep prototype chain at scale (runs in both modes), plus a scoped for-in benchmark under benchmarks/for-in/ with its own goccia.json enabling --compat-for-in-loop. Closes #809 --- benchmarks/for-in/for-in.js | 49 ++++++++++++++++++ benchmarks/for-in/goccia.json | 3 ++ source/units/Goccia.Evaluator.pas | 12 +++-- source/units/Goccia.VM.pas | 13 +++-- .../language/for-in-loop/basic-enumeration.js | 50 +++++++++++++++++++ 5 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 benchmarks/for-in/for-in.js create mode 100644 benchmarks/for-in/goccia.json diff --git a/benchmarks/for-in/for-in.js b/benchmarks/for-in/for-in.js new file mode 100644 index 000000000..3de1496a5 --- /dev/null +++ b/benchmarks/for-in/for-in.js @@ -0,0 +1,49 @@ +/*--- +description: for...in enumeration and prototype-chain key dedup benchmarks +---*/ + +suite("for...in", () => { + // Build a `levels`-deep prototype chain where every level redeclares the + // same `width` keys (plus one unique key per level). for...in must dedup + // the shared keys down the chain, which stresses the key-dedup set. + const buildChain = (levels, width) => { + const sharedKeys = Array.from({ length: width }, (_, i) => "k" + i); + return Array.from({ length: levels }).reduce((proto, _unused, level) => { + const obj = Object.create(proto); + for (const key of sharedKeys) obj[key] = level; + obj["unique" + level] = level; + return obj; + }, null); + }; + + const flat50 = Array.from({ length: 50 }).reduce((obj, _unused, i) => { + obj["k" + i] = i; + return obj; + }, {}); + + bench("for...in over 50 own keys", { + run: () => { + let count = 0; + for (const key in flat50) count = count + 1; + return count; + }, + }); + + bench("for...in over an 8-level chain of 50 shared keys", { + setup: () => buildChain(8, 50), + run: (obj) => { + let count = 0; + for (const key in obj) count = count + 1; + return count; + }, + }); + + bench("for...in over a 16-level chain of 100 shared keys", { + setup: () => buildChain(16, 100), + run: (obj) => { + let count = 0; + for (const key in obj) count = count + 1; + return count; + }, + }); +}); diff --git a/benchmarks/for-in/goccia.json b/benchmarks/for-in/goccia.json new file mode 100644 index 000000000..4d373517d --- /dev/null +++ b/benchmarks/for-in/goccia.json @@ -0,0 +1,3 @@ +{ + "compat-for-in-loop": true +} diff --git a/source/units/Goccia.Evaluator.pas b/source/units/Goccia.Evaluator.pas index 1235ab28b..7468a82ad 100644 --- a/source/units/Goccia.Evaluator.pas +++ b/source/units/Goccia.Evaluator.pas @@ -5408,7 +5408,7 @@ function CreateForInEntriesArray(const AValue: TGocciaValue): TGocciaArrayValue; Keys: TArray; Key: string; KeyValue: TGocciaStringLiteralValue; - Visited: TStringList; + Visited: TOrderedStringMap; GC: TGarbageCollector; ChainDepth: Integer; begin @@ -5424,9 +5424,11 @@ function CreateForInEntriesArray(const AValue: TGocciaValue): TGocciaArrayValue; Obj := ToObject(AValue); if Assigned(GC) then GC.AddTempRoot(Obj); - Visited := TStringList.Create; + // Dedup keys across the prototype chain via O(1) hash-set membership + // (native case-sensitive string equality); OrderForInPropertyKeys + // (above) owns per-level enumeration order. + Visited := TOrderedStringMap.Create; try - Visited.CaseSensitive := True; Current := Obj; ChainDepth := 0; while Assigned(Current) do @@ -5439,10 +5441,10 @@ function CreateForInEntriesArray(const AValue: TGocciaValue): TGocciaArrayValue; Keys := OrderForInPropertyKeys(Current.GetAllPropertyNames); for Key in Keys do begin - if Visited.IndexOf(Key) >= 0 then + if Visited.ContainsKey(Key) then Continue; - Visited.Add(Key); + Visited.Add(Key, True); EntryObj := TGocciaObjectValue.Create; if Assigned(GC) then GC.AddTempRoot(EntryObj); diff --git a/source/units/Goccia.VM.pas b/source/units/Goccia.VM.pas index f80b26d8a..b05e37f22 100644 --- a/source/units/Goccia.VM.pas +++ b/source/units/Goccia.VM.pas @@ -469,6 +469,7 @@ implementation SysUtils, BigInteger, + OrderedStringMap, TextSemantics, TimingUtils, @@ -8425,7 +8426,7 @@ function TGocciaVM.ForInEntriesArray( Keys: TArray; Key: string; KeyValue: TGocciaStringLiteralValue; - Visited: TStringList; + Visited: TOrderedStringMap; GC: TGarbageCollector; ChainDepth: Integer; begin @@ -8441,9 +8442,11 @@ function TGocciaVM.ForInEntriesArray( Obj := ToObject(AValue); if Assigned(GC) then GC.AddTempRoot(Obj); - Visited := TStringList.Create; + // Dedup keys across the prototype chain via O(1) hash-set membership + // (native case-sensitive string equality); VMOrderOwnPropertyStringKeys + // (above) owns per-level enumeration order. + Visited := TOrderedStringMap.Create; try - Visited.CaseSensitive := True; Current := Obj; ChainDepth := 0; while Assigned(Current) do @@ -8456,10 +8459,10 @@ function TGocciaVM.ForInEntriesArray( Keys := VMOrderOwnPropertyStringKeys(Current.GetAllPropertyNames); for Key in Keys do begin - if Visited.IndexOf(Key) >= 0 then + if Visited.ContainsKey(Key) then Continue; - Visited.Add(Key); + Visited.Add(Key, True); EntryObj := TGocciaObjectValue.Create; if Assigned(GC) then GC.AddTempRoot(EntryObj); diff --git a/tests/language/for-in-loop/basic-enumeration.js b/tests/language/for-in-loop/basic-enumeration.js index 865d3d2e8..169eea37f 100644 --- a/tests/language/for-in-loop/basic-enumeration.js +++ b/tests/language/for-in-loop/basic-enumeration.js @@ -151,3 +151,53 @@ test("deleted own shadow does not expose inherited property", () => { expect(keys).toEqual(["a"]); }); + +test("enumerable own key shadowing a same-named inherited key is yielded once", () => { + const proto = { shared: "proto", onlyProto: 1 }; + const obj = Object.create(proto); + obj.shared = "own"; + obj.onlyOwn = 2; + + const keys = []; + for (const key in obj) keys.push(key); + + // "shared" appears once, in its own (nearer) position; never re-yielded + // from the prototype. + expect(keys).toEqual(["shared", "onlyOwn", "onlyProto"]); +}); + +test("dedups same-named keys across a deep prototype chain at scale", () => { + // Build a 12-level chain where every level redeclares the same 40 keys, + // plus one level-unique key. Each shared key must be yielded exactly once + // (from the nearest level); per-level order is preserved. + const LEVELS = 12; + const SHARED = 40; + const sharedKeys = Array.from({ length: SHARED }, (_, i) => "k" + i); + + const leaf = Array.from({ length: LEVELS }).reduce((proto, _unused, level) => { + const obj = Object.create(proto); + for (const key of sharedKeys) obj[key] = level; + obj["unique" + level] = level; + return obj; + }, null); + + const seen = []; + const counts = {}; + for (const key in leaf) { + seen.push(key); + counts[key] = (counts[key] || 0) + 1; + } + + // Every key yielded exactly once. + for (const key of seen) expect(counts[key]).toBe(1); + + // The nearest (leaf) level owns the shared keys, in their declared order, + // before any inherited level-unique keys. + expect(seen.slice(0, SHARED)).toEqual(sharedKeys); + + // All shared keys plus one unique key per level, no duplicates. + expect(seen.length).toBe(SHARED + LEVELS); + + // Shared keys resolve to the leaf level's value. + expect(leaf.k0).toBe(LEVELS - 1); +}); From 53193240f32e7eeb81239f6cbe4ef04404959722 Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 27 Jun 2026 19:27:59 +0100 Subject: [PATCH 2/2] test(for-in): assert inherited unique-key suffix order in dedup test Strengthen the deep-chain for-in dedup test to also assert the order of the inherited level-unique keys (leaf to root), not just the shared-key prefix and total count, so a prototype-level enumeration-order regression cannot slip through. --- tests/language/for-in-loop/basic-enumeration.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/language/for-in-loop/basic-enumeration.js b/tests/language/for-in-loop/basic-enumeration.js index 169eea37f..d1ab78468 100644 --- a/tests/language/for-in-loop/basic-enumeration.js +++ b/tests/language/for-in-loop/basic-enumeration.js @@ -198,6 +198,11 @@ test("dedups same-named keys across a deep prototype chain at scale", () => { // All shared keys plus one unique key per level, no duplicates. expect(seen.length).toBe(SHARED + LEVELS); + // The inherited level-unique keys follow, ordered nearest (leaf) to + // farthest — one per level, guarding against prototype-level reordering. + const expectedUnique = Array.from({ length: LEVELS }, (_, i) => "unique" + (LEVELS - 1 - i)); + expect(seen.slice(SHARED)).toEqual(expectedUnique); + // Shared keys resolve to the leaf level's value. expect(leaf.k0).toBe(LEVELS - 1); });