Skip to content

LinkedDictionary: mode-aware ordering — pure reads in Dynamic, fail-fast LRU in Fixed - #16

Open
Codeturion wants to merge 3 commits into
mainfrom
fix/linkeddictionary-read-reorder
Open

LinkedDictionary: mode-aware ordering — pure reads in Dynamic, fail-fast LRU in Fixed#16
Codeturion wants to merge 3 commits into
mainfrom
fix/linkeddictionary-read-reorder

Conversation

@Codeturion

Copy link
Copy Markdown
Owner

Addresses external feedback that LinkedDictionary mutates its ordering chain on every lookup.

Problem

  • TryGetValue / this[key] get called MoveToFront on every hit — every read was a write, contradicting the doc claim of preserved insertion order.
  • MoveToFront did not bump _version, so lookups during foreach silently corrupted iteration (skipped/repeated items) instead of throwing.
  • ContainsKey didn't reorder while TryGetValue did — inconsistent read semantics.
  • In Dynamic mode (no eviction) the reorder-on-read was pure overhead.

Change

  • Dynamic mode (default): reads are pure — no reorder on TryGetValue/indexer get; AddOrUpdate on an existing key keeps its insertion position. Iteration order is true insertion order.
  • Fixed mode (LRU cache): lookups and updates still move the entry to MRU (that's the point), and the reorder now bumps _version so an active enumerator fails fast instead of silently corrupting.
  • Class XML doc + README updated to state the per-mode semantics.
  • Tests: 3 existing LRU tests moved to Fixed mode; 4 new tests covering Dynamic purity, insertion-position stability, safe lookups during Dynamic enumeration, and Fixed-mode fail-fast.

Breaking

Behavioral change for anyone relying on LRU reordering in Dynamic mode — that usage now belongs in Fixed mode. ConcurrentLinkedDictionary (timestamp-based) and BoundedDictionary are unaffected; LinkedMultiMap still reorders on read but documents it (possible follow-up).

…rder with pure reads, Fixed reorders on access and fail-fasts enumerators

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a CapacityMode to LinkedDictionary, separating its behavior into Dynamic (preserving insertion order with pure reads) and Fixed (acting as an LRU cache). The implementation of AddOrUpdate and TryGetValue has been updated to only reorder elements when in Fixed mode, and several new tests have been added to verify these behaviors. The review feedback correctly identifies that the newly added tests assert the incorrect enumeration order (expecting insertion order instead of reverse insertion order/newest-to-oldest), which will cause test failures, and provides code suggestions to fix these assertions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Omni.Collections.Tests/Hybrid/LinkedDictionaryTests.cs
Comment thread src/Omni.Collections.Tests/Hybrid/LinkedDictionaryTests.cs
Comment thread src/Omni.Collections.Tests/Hybrid/LinkedDictionaryTests.cs
@Codeturion

Copy link
Copy Markdown
Owner Author

Follow-up in 130e051: the enumerator walked head→tail, which is MRU-first (reverse insertion) — the old "insertion order" test even asserted third, second, first against its own docstring. Enumeration now starts at the tail and walks PrevOrdering: insertion order in Dynamic mode, LRU→MRU in Fixed mode (Java LinkedHashMap semantics). Existing enumeration-order test updated to oldest-first.

@Codeturion

Copy link
Copy Markdown
Owner Author

The three gemini-code-assist comments describe the pre-130e051 behavior. That commit changed the enumerator to walk tail→head, so enumeration is now oldest-first (insertion order in Dynamic mode, LRU→MRU in Fixed mode) and Enumeration_MultipleItems_TraversesInInsertionOrder was updated to first, second, third in the same commit. The flagged assertions pass on current HEAD — ubuntu-latest is green on all three seeds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant