Various improvements #169

Merged
schmelczer merged 78 commits from asch/saturday into main 2025-11-30 15:24:52 +00:00
3 changed files with 28 additions and 6 deletions
Showing only changes of commit 088fad734a - Show all commits

Fix edge cases

Andras Schmelczer 2025-11-22 20:14:31 +00:00

View file

@ -49,14 +49,17 @@ export class Locks<T> {
fn: () => R | Promise<R> fn: () => R | Promise<R>
): Promise<R> { ): Promise<R> {
const keys = Array.isArray(keyOrKeys) ? keyOrKeys : [keyOrKeys]; const keys = Array.isArray(keyOrKeys) ? keyOrKeys : [keyOrKeys];
keys.sort((a, b) => String(a).localeCompare(String(b))); // Ensure consistent order to prevent deadlocks
await Promise.all(keys.map(async (key) => this.waitForLock(key))); // Deduplicate keys to prevent deadlock from acquiring same lock twice
const uniqueKeys = Array.from(new Set(keys));
uniqueKeys.sort((a, b) => String(a).localeCompare(String(b))); // Ensure consistent order to prevent deadlocks
await Promise.all(uniqueKeys.map(async (key) => this.waitForLock(key)));
try { try {
return await fn(); return await fn();
} finally { } finally {
keys.forEach((key) => { uniqueKeys.forEach((key) => {
this.unlock(key); this.unlock(key);
}); });
} }

View file

@ -48,15 +48,29 @@ describe("CoveredValues", () => {
assert.strictEqual(covered.min, 6); assert.strictEqual(covered.min, 6);
}); });
it("should handle force setting min value", () => { it("should auto-advance when setting min value", () => {
const covered = new CoveredValues(5); const covered = new CoveredValues(5);
covered.add(7); covered.add(7);
covered.add(8); covered.add(8);
covered.add(9); covered.add(9);
assert.strictEqual(covered.min, 5); assert.strictEqual(covered.min, 5);
// Setting min to 6 should auto-advance through 7, 8, 9
covered.min = 6; covered.min = 6;
assert.strictEqual(covered.min, 6); assert.strictEqual(covered.min, 9);
covered.add(10); covered.add(10);
assert.strictEqual(covered.min, 10); assert.strictEqual(covered.min, 10);
}); });
it("should handle setting min value with no consecutive values", () => {
const covered = new CoveredValues(5);
covered.add(10);
covered.add(15);
assert.strictEqual(covered.min, 5);
// Setting min to 8 should not auto-advance (no consecutive values)
covered.min = 8;
assert.strictEqual(covered.min, 8);
// Add 9 to trigger auto-advance to 10
covered.add(9);
assert.strictEqual(covered.min, 10);
});
}); });

View file

@ -24,7 +24,8 @@ export class CoveredValues {
public set min(value: number) { public set min(value: number) {
this.minValue = Math.max(value, this.minValue); this.minValue = Math.max(value, this.minValue);
this.seenValues = this.seenValues.filter((v) => v > value); this.seenValues = this.seenValues.filter((v) => v > this.minValue);
this.advanceMinWhilePossible();
copilot-pull-request-reviewer[bot] commented 2025-11-27 22:26:20 +00:00 (Migrated from github.com)

The filter should compare against value (the input parameter) rather than this.minValue since this.minValue is updated on line 26 and may not reflect the original intent. If the goal is to filter values greater than the new minimum, this is correct but unclear.

The filter should compare against `value` (the input parameter) rather than `this.minValue` since `this.minValue` is updated on line 26 and may not reflect the original intent. If the goal is to filter values greater than the new minimum, this is correct but unclear.
} }
copilot-pull-request-reviewer[bot] commented 2025-11-30 15:03:02 +00:00 (Migrated from github.com)

Filtering the entire seenValues array on every min update could be inefficient for large arrays. Since seenValues is sorted, consider using binary search to find the first value > this.minValue and slicing from that index instead of filtering.

Filtering the entire seenValues array on every min update could be inefficient for large arrays. Since seenValues is sorted, consider using binary search to find the first value > this.minValue and slicing from that index instead of filtering.
public add(value: number): void { public add(value: number): void {
@ -45,6 +46,10 @@ export class CoveredValues {
this.seenValues.splice(i, 0, value); this.seenValues.splice(i, 0, value);
} }
this.advanceMinWhilePossible();
}
private advanceMinWhilePossible(): void {
while ( while (
this.seenValues.length > 0 && this.seenValues.length > 0 &&
this.seenValues[0] === this.minValue + 1 this.seenValues[0] === this.minValue + 1