Skip to content

Fix cache eviction order, redundant matchesQuery recursion, and sync/async misuse #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hexagon merged 2 commits into from
Feb 20, 2026
Merged

Fix cache eviction order, redundant matchesQuery recursion, and sync/async misuse #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f87071
Initial plan
Copilot Feb 20, 2026
37fb02c
Fix performance and architectural issues: cache eviction order, index…
Copilot Feb 20, 2026
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
8 changes: 4 additions & 4 deletions src/lib/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import type { KVLedgerResult } from "./ledger.ts";
* Since offsets are numeric, objects provide faster lookups (O(1)) compared to `Map`
* in this specific use case.
*
* Additionally, an array of inserted offsets is used to provide O(1) lookups of offsets
* to evict.
* An array of inserted offsets in insertion order is maintained to support
* FIFO eviction: the oldest (first-inserted) entry is evicted when the cache is full.
*/
export class KVLedgerCache {
private cache: Record<number, KVLedgerResult> = {};
Expand Down Expand Up @@ -71,8 +71,8 @@ export class KVLedgerCache {
*/
private evictOldestEntries(): void {
while (this.cacheSizeBytes > this.maxCacheSizeBytes) {
const oldestOffset = this.cacheEntries.pop() as number;
if (oldestOffset) {
const oldestOffset = this.cacheEntries.shift();
if (oldestOffset !== undefined) {
const oldestData = this.cache[oldestOffset];
delete this.cache[oldestOffset];
this.cacheSizeBytes -= oldestData.length * LEDGER_CACHE_MEMORY_FACTOR;
Expand Down
5 changes: 3 additions & 2 deletions src/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ export class KVIndex {
*/
get(key: KVKeyInstance, limit?: number, reverse: boolean = false): number[] {
const resultSet: number[] = [];
const keyLength = key.get().length;
const keyParts = key.get();
const keyLength = keyParts.length;

function recurse(node: KVIndexContent, keyIndex: number): void {
if (keyIndex >= keyLength) {
Expand All @@ -146,7 +147,7 @@ export class KVIndex {
return;
}

const keyPart = key.get()[keyIndex];
const keyPart = keyParts[keyIndex];

if (typeof keyPart === "string" || typeof keyPart === "number") {
// Standard string/number part
Expand Down
14 changes: 0 additions & 14 deletions src/lib/key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,20 +300,6 @@ export class KVKeyInstance {
} else {
throw new Error(`Invalid query element type at index ${i}`);
}

// Recursively check descendants if needed
if (recursive && thisKey.length > i + 1) {
const subquery = query.slice(i + 1);
const subkey = thisKey.slice(i + 1);
if (
!new KVKeyInstance(subkey, true, false).matchesQuery(
subquery,
recursive,
)
) {
return false;
}
}
}

return true; // All elements match
Expand Down
4 changes: 2 additions & 2 deletions src/lib/kv.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ export class KV extends EventEmitter {
const hashAlgo = this.ledger?.header.ledgerVersion === "B016"
? KVHashAlgorithm.FAULTY_MURMURHASH3
: KVHashAlgorithm.MURMURHASH3;
await transaction.create(
transaction.create(
validatedKey,
KVOperation.SET,
Date.now(),
Expand Down Expand Up @@ -809,7 +809,7 @@ export class KV extends EventEmitter {
// Prepare transaction data and offsets
let currentOffset = 0;
for (const transaction of this.pendingTransactions) {
const transactionData = await transaction.toUint8Array();
const transactionData = transaction.toUint8Array();
bufferedTransactions.push({
transaction,
transactionData,
Expand Down
63 changes: 31 additions & 32 deletions test/cache.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,11 @@ test("KVLedgerCache: eviction when size exceeded", () => {
cache.cacheTransactionData(300, result3);
cache.cacheTransactionData(400, result4); // Should trigger eviction

// Assert: Fourth entry should be evicted (pop() removes from end)
// The implementation uses pop() which removes the last pushed item
assertEquals(cache.getTransactionData(100) !== undefined, true);
// Assert: First (oldest) entry should be evicted (shift() removes from front)
assertEquals(cache.getTransactionData(100), undefined); // Evicted (oldest)
assertEquals(cache.getTransactionData(200) !== undefined, true);
assertEquals(cache.getTransactionData(300) !== undefined, true);
assertEquals(cache.getTransactionData(400), undefined); // Evicted
assertEquals(cache.getTransactionData(400) !== undefined, true);
});

test("KVLedgerCache: multiple evictions when needed", () => {
Expand All @@ -87,13 +86,13 @@ test("KVLedgerCache: multiple evictions when needed", () => {

// Act: Add entries that require evictions
cache.cacheTransactionData(100, result1); // 30 bytes, ok
cache.cacheTransactionData(200, result2); // 60 bytes > 50, evict 200
cache.cacheTransactionData(300, result3); // 60 bytes > 50, evict 300
cache.cacheTransactionData(200, result2); // 60 bytes > 50, evict oldest (100)
cache.cacheTransactionData(300, result3); // 60 bytes > 50, evict oldest (200)

// Assert: Only first entry remains (implementation evicts newest with pop())
assertEquals(cache.getTransactionData(100) !== undefined, true);
// Assert: Only the most-recently added entry remains (eviction removes oldest with shift())
assertEquals(cache.getTransactionData(100), undefined);
assertEquals(cache.getTransactionData(200), undefined);
assertEquals(cache.getTransactionData(300), undefined);
assertEquals(cache.getTransactionData(300) !== undefined, true);
});

test("KVLedgerCache: clear removes all entries", () => {
Expand Down Expand Up @@ -141,11 +140,11 @@ test("KVLedgerCache: respects max cache size", () => {
);
}

// Assert: Only first entries should remain (eviction removes from end with pop())
assertEquals(cache.getTransactionData(0) !== undefined, true);
assertEquals(cache.getTransactionData(100) !== undefined, true);
assertEquals(cache.getTransactionData(200) !== undefined, true);
assertEquals(cache.getTransactionData(900), undefined);
// Assert: Only last entries should remain (eviction removes oldest with shift())
assertEquals(cache.getTransactionData(0), undefined);
assertEquals(cache.getTransactionData(100), undefined);
assertEquals(cache.getTransactionData(200), undefined);
assertEquals(cache.getTransactionData(900) !== undefined, true);
});

test("KVLedgerCache: handles zero-length transactions", () => {
Expand Down Expand Up @@ -175,7 +174,7 @@ test("KVLedgerCache: handles large transactions", () => {
assertEquals(retrieved, result);
});

test("KVLedgerCache: eviction order uses pop (removes newest)", () => {
test("KVLedgerCache: eviction order uses shift (removes oldest)", () => {
// Arrange: Cache that can hold about 3 entries
// Each entry with length=15 takes 15*3=45 bytes
// 150 byte cache can hold about 3 entries (135 bytes)
Expand All @@ -187,11 +186,11 @@ test("KVLedgerCache: eviction order uses pop (removes newest)", () => {
cache.cacheTransactionData(300, createMockLedgerResult(300, 15, "third"));
cache.cacheTransactionData(400, createMockLedgerResult(400, 15, "fourth")); // Exceeds, triggers eviction

// Assert: Implementation uses pop() which evicts newest (last pushed)
// Assert: Implementation uses shift() which evicts oldest (first pushed)
assertEquals(
cache.getTransactionData(100) !== undefined,
true,
"First should remain",
cache.getTransactionData(100),
undefined,
"First (oldest) should be evicted",
);
assertEquals(
cache.getTransactionData(200) !== undefined,
Expand All @@ -204,9 +203,9 @@ test("KVLedgerCache: eviction order uses pop (removes newest)", () => {
"Third should remain",
);
assertEquals(
cache.getTransactionData(400),
undefined,
"Fourth should be evicted",
cache.getTransactionData(400) !== undefined,
true,
"Fourth should remain",
);
});

Expand All @@ -224,12 +223,12 @@ test("KVLedgerCache: get after eviction returns undefined", () => {
// Assert: First entry should be cached
assertEquals(cache.getTransactionData(100) !== undefined, true);

// Act: Try to cache second entry which causes eviction of second (pop removes newest)
cache.cacheTransactionData(200, result2); // 180 bytes > 100, evicts 200
// Act: Cache second entry which causes eviction of first (shift removes oldest)
cache.cacheTransactionData(200, result2); // 180 bytes > 100, evicts 100

// Assert: Second entry should be evicted immediately (pop removes newest)
assertEquals(cache.getTransactionData(100) !== undefined, true);
assertEquals(cache.getTransactionData(200), undefined);
// Assert: First entry should be evicted (shift removes oldest), second remains
assertEquals(cache.getTransactionData(100), undefined);
assertEquals(cache.getTransactionData(200) !== undefined, true);
});

test("KVLedgerCache: handles rapid cache/evict cycles", () => {
Expand All @@ -243,11 +242,11 @@ test("KVLedgerCache: handles rapid cache/evict cycles", () => {
cache.cacheTransactionData(i, createMockLedgerResult(i, 10, `test${i}`));
}

// Assert: Only first 3 entries should remain (eviction removes newest with pop())
assertEquals(cache.getTransactionData(0) !== undefined, true);
assertEquals(cache.getTransactionData(1) !== undefined, true);
assertEquals(cache.getTransactionData(2) !== undefined, true);
assertEquals(cache.getTransactionData(99), undefined);
// Assert: Only last 3 entries should remain (eviction removes oldest with shift())
assertEquals(cache.getTransactionData(0), undefined);
assertEquals(cache.getTransactionData(97) !== undefined, true);
assertEquals(cache.getTransactionData(98) !== undefined, true);
assertEquals(cache.getTransactionData(99) !== undefined, true);
});

test("KVLedgerCache: clear resets size tracking", () => {
Expand Down
Loading