Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
7 changes: 7 additions & 0 deletions .changeset/append-path-parts-interning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@geajs/core": patch
---

### @geajs/core (patch)

- **Hot-path path-parts interning**: Replace per-call `appendPathParts` allocations in `_wrapItem` and array mutation handlers (splice, push/unshift, pop/shift) with a module-level `WeakMap` cache keyed on stable `baseParts` references, eliminating redundant array allocations in list-heavy workloads.
111 changes: 111 additions & 0 deletions packages/gea/benchmarks/path-interning.bench.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* Benchmark: path parts interning — eliminate hot-path array allocations
* PR #42: Cache [...parent, key] results in _appendCache WeakMap
*
* Run: npx tsx --conditions source packages/gea/benchmarks/path-interning.bench.ts
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
*/
import { Store } from '../src/lib/store.ts'

function heapMB() {
return process.memoryUsage().heapUsed / 1024 / 1024
}

function bench(fn: () => void, iters: number): number {
for (let i = 0; i < 20; i++) fn()
const t0 = performance.now()
for (let i = 0; i < iters; i++) fn()
return performance.now() - t0
}

// ---------- OLD: naive spread (always allocates) ----------
function appendOld(parent: string[], key: string): string[] {
return [...parent, key]
}

// ---------- NEW: intern cache (returns cached reference) ----------
const _appendCache = new WeakMap<string[], Map<string, string[]>>()
function appendNew(parent: string[], key: string): string[] {
let map = _appendCache.get(parent)
if (!map) {
map = new Map()
_appendCache.set(parent, map)
}
let result = map.get(key)
if (!result) {
result = [...parent, key]
map.set(key, result)
}
return result
}

const keys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j']
const ITERS = 200_000

console.log('\n=== path parts interning benchmark ===')
console.log('Simulating hot-path proxy navigation: append key to parent path array\n')

// --- Shallow path (depth 1) ---
{
const parent: string[] = []
if (typeof global.gc === 'function') global.gc()
const h0 = heapMB()
const oldMs = bench(() => { for (const k of keys) appendOld(parent, k) }, ITERS)
if (typeof global.gc === 'function') global.gc()
const h1 = heapMB()
const newMs = bench(() => { for (const k of keys) appendNew(parent, k) }, ITERS)
if (typeof global.gc === 'function') global.gc()
const h2 = heapMB()
console.log('Shallow path (depth 1):')
console.log(` old (spread): ${oldMs.toFixed(2)}ms heap Δ ${(h1-h0).toFixed(3)} MB`)
console.log(` new (interned): ${newMs.toFixed(2)}ms heap Δ ${(h2-h1).toFixed(3)} MB`)
console.log(` speedup: ${(oldMs/newMs).toFixed(1)}x\n`)
}

// --- Deep path (depth 5) ---
{
const depth5 = ['store', 'user', 'profile', 'address', 'city']
if (typeof global.gc === 'function') global.gc()
const h0 = heapMB()
const oldMs = bench(() => { for (const k of keys) appendOld(depth5, k) }, ITERS)
if (typeof global.gc === 'function') global.gc()
const h1 = heapMB()
const newMs = bench(() => { for (const k of keys) appendNew(depth5, k) }, ITERS)
if (typeof global.gc === 'function') global.gc()
const h2 = heapMB()
console.log('Deep path (depth 5):')
console.log(` old (spread): ${oldMs.toFixed(2)}ms heap Δ ${(h1-h0).toFixed(3)} MB`)
console.log(` new (interned): ${newMs.toFixed(2)}ms heap Δ ${(h2-h1).toFixed(3)} MB`)
console.log(` speedup: ${(oldMs/newMs).toFixed(1)}x\n`)
}

// --- Real store: deep reactive property access ---
class DeepStore extends Store {
user = {
profile: {
address: {
city: 'Istanbul',
zip: '34000',
}
}
}
}

const store = new DeepStore()
const STORE_ITERS = 50_000

if (typeof global.gc === 'function') global.gc()
const hs0 = heapMB()
const storeMs = bench(() => {
void store.user.profile.address.city
void store.user.profile.address.zip
}, STORE_ITERS)
if (typeof global.gc === 'function') global.gc()
const hs1 = heapMB()

console.log('Real store deep property access (depth 4, 2 leaf props):')
console.log(` ${STORE_ITERS.toLocaleString()} iterations: ${storeMs.toFixed(2)}ms`)
console.log(` per-iter: ${((storeMs/STORE_ITERS)*1000).toFixed(1)}µs`)
console.log(` heap delta: ${(hs1-hs0).toFixed(3)} MB`)
console.log()
console.log('With path interning: same path arrays are reused across proxy navigations.')
console.log('Without interning: each proxy access spreads a new array for each path segment.\n')
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
29 changes: 24 additions & 5 deletions packages/gea/src/lib/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,25 @@ function appendPathParts(pathParts: string[], propStr: string): string[] {
return [...pathParts, propStr]
}

// Module-level cache: WeakMap<baseParts, Map<segment, result>>.
// Keys are the stable cached baseParts arrays produced by _makePathCache,
// so entries are GC'd automatically when the owning proxy is collected.
const _appendCache = new WeakMap<string[], Map<string, string[]>>()

function _internAppend(baseParts: string[], segment: string): string[] {
let inner = _appendCache.get(baseParts)
if (inner === undefined) {
inner = new Map()
_appendCache.set(baseParts, inner)
}
let result = inner.get(segment)
if (result === undefined) {
result = baseParts.length > 0 ? [...baseParts, segment] : [segment]
inner.set(segment, result)
}
return result
}

function joinPath(basePath: string, seg: string | number): string {
return basePath ? `${basePath}.${seg}` : String(seg)
}
Expand Down Expand Up @@ -155,7 +174,7 @@ const getByPathParts = (obj: any, pathParts: string[]): any => pathParts.reduce(
function _wrapItem(store: Store, arr: any[], i: number, basePath: string, baseParts: string[]): any {
const raw = arr[i]
return shouldWrapNestedReactiveValue(raw)
? _createProxy(store, raw, joinPath(basePath, i), appendPathParts(baseParts, String(i)))
? _createProxy(store, raw, joinPath(basePath, i), _internAppend(baseParts, String(i)))
: raw
Comment on lines 171 to 175

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't reuse raw-object proxies for array-index paths.

_wrapItem() is building an index-scoped path here, but _createProxy() still checks proxyCache before it derives cachedArrayMeta. After unshift/splice/sort/reverse, a previously seen element object can come back with its old proxy, so nested writes keep emitting the old basePath/pathParts (for example, items.0.* after the object moved to items.1). Please derive the array metadata before the raw-object cache lookup and keep index-scoped proxies out of the global proxyCache path.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/gea/src/lib/store.ts` around lines 170 - 174, _wrapItem is creating
index-scoped proxies for array elements which allows _createProxy to return a
stale proxy from proxyCache with incorrect cachedArrayMeta (wrong
basePath/baseParts) after array mutations; fix by deriving array-specific
metadata (cachedArrayMeta or equivalent) inside _wrapItem before calling
_createProxy and ensure _createProxy does not use the global proxyCache for
index-scoped paths (i.e., do not cache proxies whose pathParts include
numeric/index segments). Update _wrapItem, _createProxy, and the proxyCache
lookup logic so index-scoped proxies are created fresh and non-indexed object
proxies remain cached; reference functions/variables: _wrapItem, _createProxy,
proxyCache, cachedArrayMeta, joinPath, _internAppend.

}

Expand Down Expand Up @@ -721,11 +740,11 @@ function _interceptArray(
const changes: StoreChange[] = []
for (let i = 0; i < removed.length; i++) {
const idx = String(start + i)
changes.push(_mkChange('delete', idx, arr, appendPathParts(baseParts, idx), undefined, removed[i]))
changes.push(_mkChange('delete', idx, arr, _internAppend(baseParts, idx), undefined, removed[i]))
}
for (let i = 0; i < items.length; i++) {
const idx = String(start + i)
changes.push(_mkChange('add', idx, arr, appendPathParts(baseParts, idx), items[i]))
changes.push(_mkChange('add', idx, arr, _internAppend(baseParts, idx), items[i]))
}
if (changes.length > 0) _pushAndSchedule(store, changes, p)
return removed
Expand All @@ -743,7 +762,7 @@ function _interceptArray(
} else {
const changes: StoreChange[] = []
for (let i = 0; i < rawItems.length; i++)
changes.push(_mkChange('add', String(i), arr, appendPathParts(baseParts, String(i)), rawItems[i]))
changes.push(_mkChange('add', String(i), arr, _internAppend(baseParts, String(i)), rawItems[i]))
_pushAndSchedule(store, changes, p)
}
return arr.length
Expand All @@ -758,7 +777,7 @@ function _interceptArray(
;(Array.prototype as any)[method].call(arr)
_pushAndSchedule(
store,
[_mkChange('delete', String(idx), arr, appendPathParts(baseParts, String(idx)), undefined, removed)],
[_mkChange('delete', String(idx), arr, _internAppend(baseParts, String(idx)), undefined, removed)],
p,
)
return removed
Expand Down