Skip to content

Commit fb2a9cd

Browse files
JoshRosenclaude
andauthored
Fix O(n^2) performance bug in uniqArr; avoid duplicate keyF evaluations (#575)
This PR implements two performance optimizations in `uniqueArr`: - **Fix an O(n^2) performance bug**: #564 changed this code to call `out.result()` on every loop iteration, resulting in copying of an `O(n)` sized array on each loop iteration. We can avoid this by keeping a reference to the last output element. - **Avoid duplicate keyF evaluations**: in the old code, each loop iteration would repeat the `keyF` evaluation for the last element of the output array; the new code calls `keyF` exactly once per element by maintaining a `lastAddedKey` variable during the loop. - This is similar in spirit to #245 Claude (Opus 4.5) spotted the O(n^2) bug and designed that part of the fix. I spotted the duplicate keyF evaluation and suggested this fix (plus the switch to a `while` loop). Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 4ab8288 commit fb2a9cd

1 file changed

Lines changed: 11 additions & 14 deletions

File tree

sjsonnet/src/sjsonnet/stdlib/SetModule.scala

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,22 +65,19 @@ object SetModule extends AbstractFunctionModule {
6565
val out = new mutable.ArrayBuilder.ofRef[Lazy]
6666
// Set a reasonable size hint - in the worst case (no duplicates), we'll need arrValue.length elements
6767
out.sizeHint(arrValue.length)
68-
for (v <- arrValue) {
69-
val outResult = out.result()
70-
if (outResult.length == 0) {
68+
69+
var lastAddedKey: Val = null
70+
var i = 0
71+
while (i < arrValue.length) {
72+
val v = arrValue(i)
73+
val vKey =
74+
if (keyF.isInstanceOf[Val.False]) v.force
75+
else keyF.asInstanceOf[Val.Func].apply1(v, pos.noOffset)(ev, TailstrictModeDisabled)
76+
if (lastAddedKey == null || !ev.equal(vKey, lastAddedKey)) {
7177
out.+=(v)
72-
} else if (keyF.isInstanceOf[Val.False]) {
73-
if (!ev.equal(outResult.last.force, v.force)) {
74-
out.+=(v)
75-
}
76-
} else if (!keyF.isInstanceOf[Val.False]) {
77-
val keyFFunc = keyF.asInstanceOf[Val.Func]
78-
val o1Key = keyFFunc.apply1(v, pos.noOffset)(ev, TailstrictModeDisabled)
79-
val o2Key = keyFFunc.apply1(outResult.last, pos.noOffset)(ev, TailstrictModeDisabled)
80-
if (!ev.equal(o1Key, o2Key)) {
81-
out.+=(v)
82-
}
78+
lastAddedKey = vKey
8379
}
80+
i += 1
8481
}
8582

8683
Val.Arr(pos, out.result())

0 commit comments

Comments
 (0)