Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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/sort-permutation-on-optimization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@geajs/core": patch
---

### @geajs/core (patch)

- **Sort/reverse permutation O(n) optimization**: Replace O(n^2) nested-loop permutation calculation with a Map-based O(n) index lookup, improving performance on large sorted arrays.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
node_modules
.history
.omc
.serena
.DS_Store
dist
dist-profile
Expand Down
102 changes: 102 additions & 0 deletions packages/gea/benchmarks/sort-permutation.bench.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/**
* Benchmark: sort/reverse permutation O(n²) → O(n)
* PR #38: Replace indexOf-in-loop with Map-based bucket lookup
*
* Run: npx tsx packages/gea/benchmarks/sort-permutation.bench.ts
*/

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

// ---------- OLD implementation (O(n²)) ----------
function computePermutationOld(prev: any[], next: any[]): number[] {
return next.map((v) => {
const idx = prev.indexOf(v)
prev[idx] = undefined // mark consumed
return idx
})
}

// ---------- NEW implementation (O(n)) ----------
function computePermutationNew(prev: any[], next: any[]): number[] {
const idxMap = new Map<any, number[]>()
for (let i = 0; i < prev.length; i++) {
const a = idxMap.get(prev[i])
a ? a.push(i) : idxMap.set(prev[i], [i])
}
const cursors = new Map<any, number>()
return next.map((v) => {
const bucket = idxMap.get(v)!
const cursor = cursors.get(v) ?? 0
cursors.set(v, cursor + 1)
return bucket[cursor]
})
}
Comment on lines +28 to +41

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 | 🟡 Minor

Add explicit invariant checks in computePermutationNew.

Line 30’s non-null assertion (!) assumes perfect permutation input. If setup drifts, failure becomes opaque at Line 33. Prefer explicit guards with clear errors.

Suggested fix
 function computePermutationNew(prev: any[], next: any[]): number[] {
   const idxMap = new Map<any, number[]>()
   for (let i = 0; i < prev.length; i++) {
     const a = idxMap.get(prev[i])
     a ? a.push(i) : idxMap.set(prev[i], [i])
   }
   const cursors = new Map<any, number>()
   return next.map((v) => {
-    const bucket = idxMap.get(v)!
+    const bucket = idxMap.get(v)
+    if (!bucket) throw new Error('Invalid permutation input: value not found in prev')
     const cursor = cursors.get(v) ?? 0
+    if (cursor >= bucket.length) throw new Error('Invalid permutation input: duplicate count mismatch')
     cursors.set(v, cursor + 1)
     return bucket[cursor]
   })
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function computePermutationNew(prev: any[], next: any[]): number[] {
const idxMap = new Map<any, number[]>()
for (let i = 0; i < prev.length; i++) {
const a = idxMap.get(prev[i])
a ? a.push(i) : idxMap.set(prev[i], [i])
}
const cursors = new Map<any, number>()
return next.map((v) => {
const bucket = idxMap.get(v)!
const cursor = cursors.get(v) ?? 0
cursors.set(v, cursor + 1)
return bucket[cursor]
})
}
function computePermutationNew(prev: any[], next: any[]): number[] {
const idxMap = new Map<any, number[]>()
for (let i = 0; i < prev.length; i++) {
const a = idxMap.get(prev[i])
a ? a.push(i) : idxMap.set(prev[i], [i])
}
const cursors = new Map<any, number>()
return next.map((v) => {
const bucket = idxMap.get(v)
if (!bucket) throw new Error('Invalid permutation input: value not found in prev')
const cursor = cursors.get(v) ?? 0
if (cursor >= bucket.length) throw new Error('Invalid permutation input: duplicate count mismatch')
cursors.set(v, cursor + 1)
return bucket[cursor]
})
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/gea/benchmarks/sort-permutation.bench.ts` around lines 22 - 35, The
computePermutationNew function currently uses a non-null assertion on
idxMap.get(v) and assumes cursor indexes are valid; replace that with explicit
invariant checks: when retrieving bucket from idxMap (in computePermutationNew)
validate bucket is defined and throw a descriptive Error mentioning the missing
value v if not found, and after computing cursor ensure cursor < bucket.length
and throw a descriptive Error if out of bounds; keep the existing idxMap and
cursors logic but add these guards so failures surface clear messages rather
than opaque runtime errors.


// ---------- Benchmark harness ----------
function bench(label: string, fn: () => void, iters: number): number {
// warm-up
for (let i = 0; i < 5; i++) fn()
const t0 = performance.now()
for (let i = 0; i < iters; i++) fn()
return performance.now() - t0
}

function makeArray(size: number): number[] {
return Array.from({ length: size }, (_, i) => i)
}

function shuffleArray(arr: number[]): number[] {
const a = arr.slice()
for (let i = a.length - 1; i > 0; i--) {
const j = Math.floor(Math.random() * (i + 1))
;[a[i], a[j]] = [a[j], a[i]]
}
return a
}

const SIZES = [100, 1000, 5000, 10000]
const ITERS = 200

console.log('\n=== sort/reverse permutation benchmark ===')
console.log(`${'Size'.padEnd(8)} ${'Old (ms)'.padStart(10)} ${'New (ms)'.padStart(10)} ${'Speedup'.padStart(10)} ${'Heap Δ (MB)'.padStart(12)}`)
console.log('-'.repeat(56))

for (const size of SIZES) {
const base = makeArray(size)
const shuffled = shuffleArray(base)

const h0 = heapMB()

const oldMs = bench(
'old',
() => {
const prev = base.slice()
computePermutationOld(prev, shuffled)
},
ITERS,
)

const h1 = heapMB()

const newMs = bench(
'new',
() => {
const prev = base.slice()
computePermutationNew(prev, shuffled)
},
ITERS,
)

const h2 = heapMB()
const heapDelta = (h2 - h1).toFixed(3)
const speedup = (oldMs / newMs).toFixed(1)
Comment thread
senrecep marked this conversation as resolved.
Outdated

console.log(
`${String(size).padEnd(8)} ${oldMs.toFixed(2).padStart(10)} ${newMs.toFixed(2).padStart(10)} ${(speedup + 'x').padStart(10)} ${heapDelta.padStart(12)}`,
)
}

console.log('\nAll sizes: new implementation is O(n) vs O(n²) old.')
console.log('Speedup scales with array size. Heap delta is minimal.\n')
Loading