Skip to content

demo(16-tolist-in-loop): 16 — LINQ scan inside a per-request loop#165

Open
EricCogen wants to merge 1 commit intomainfrom
demo/16-tolist-in-loop
Open

demo(16-tolist-in-loop): 16 — LINQ scan inside a per-request loop#165
EricCogen wants to merge 1 commit intomainfrom
demo/16-tolist-in-loop

Conversation

@EricCogen
Copy link
Copy Markdown
Owner

16 — LINQ scan inside a per-request loop

Expected verdict: ❌ Fails — GauntletCI should fire GCI0044 (performance hot-path risk).

What changed

A new OrderItemDeduplicator runs a Where scan inside a foreach
over the same list, materialising it with .ToList() on each
iteration:

public sealed class OrderItemDeduplicator
{
    public IReadOnlyList<IReadOnlyList<OrderItem>> GroupBySku(Order order)
    {
        var groups = new List<IReadOnlyList<OrderItem>>();
        foreach (var item in order.Items)
        {
            var matches = order.Items
                .Where(i => i.Sku == item.Sku)
                .ToList();
            groups.Add(matches);
        }
        return groups;
    }
}

For an order with n items this is O(n²). For a single-item order it
is fine. For the larger orders the analytics team has been asking
about it falls over.

Why this is risky

  • The hot path here is the order-processing request, which already
    carries DB and payment latency. Spending O(n²) CPU on a list scan
    is exactly the kind of cost that disappears in dev and surfaces
    during a launch promo.
  • The fix is mechanical: hoist the grouping out into a single
    GroupBy (or build a Dictionary<Sku, List<OrderItem>>) before
    the loop. One pass, O(n).
  • "It's only n²" is the most common production-incident footgun
    in this codebase shape; GauntletCI flags it before the n grows.

What GauntletCI catches

GCI0044 Performance Hotpath Risk — a LINQ method (.Where(,
.Select(, .FirstOrDefault(, .Any(, .Count() on an added line
nested inside a for/foreach/while loop in a non-test, non-rule
file.

See scenarios/16-tolist-in-loop/README.md for the expected verdict.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

GauntletCI found issues in this PR. See inline comments for details.

foreach (var item in order.Items)
{
var matches = order.Items
.Where(i => i.Sku == item.Sku)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GCI0044 — Performance Hotpath Risk

LINQ query inside a loop

Evidence:

Line 13: .Where(i => i.Sku == item.Sku)

⚠️ Why it matters: LINQ queries inside loops cause repeated enumeration, yielding O(n²) or worse complexity that degrades performance at scale.

💡 Suggested action: Move the LINQ query outside the loop, pre-compute the result into a collection, or use a dictionary/lookup for O(1) access.

Confidence: Medium | Severity: Info

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e4c2ebda8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +12 to +14
var matches = order.Items
.Where(i => i.Sku == item.Sku)
.ToList();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Precompute SKU groups outside the item loop

This method runs order.Items.Where(...).ToList() inside a foreach over order.Items, which means an order with n items triggers n full list scans and n extra allocations (O(n²)). On larger orders, this creates avoidable CPU and GC pressure in the processing path; build the SKU groups once (e.g., GroupBy/dictionary) and reuse them instead of re-filtering on each iteration.

Useful? React with 👍 / 👎.

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