Skip to content

Conversation

verdie-g
Copy link
Contributor

@verdie-g verdie-g commented Jul 1, 2025

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:
        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

I faced a performance issue in prod where an endpoint takes 3 minutes to respond. The CPU is spent almost entirely in CrlCollectionAccessor.Contains. The problem is that the app is doing an Include on a collection navigation of ~300K items. We will try fixing it by making the collection a HashSet as mentioned in Relationship navigations but it's incovenient because it's breaking some code and it's hard to tell if the List's ordering is used.

In this change, I'm removing the use of the List enumerator to get an easy speed up for large collections. I've benchmarked this using this code

public class MyTests
{
    [Params(1, 10, 100, 1_000, 10_000, 100_000)]
    public int Count { get; set; }

    [GlobalSetup]
    public async Task GlobalSetup()
    {
        await using var db = new MyDbContext();
        await db.Database.EnsureDeletedAsync();
        await db.Database.EnsureCreatedAsync();
        db.Parents.Add(new Parent
        {
            Children = Enumerable.Range(0, Count).Select(x => new Child()).ToList(),
        });
        await db.SaveChangesAsync();
    }

    [Benchmark]
    public async Task Bench()
    {
        await using var db = new MyDbContext();
        await db.Parents
            .AsNoTracking()
            .AsSplitQuery()
            .Include(p => p.Children)
            .ToArrayAsync();
    }
}

Before:

Method Count Mean Error StdDev Op/s Gen0 Gen1 Allocated
Bench 1 118.4 us 2.12 us 2.27 us 8,442.7212 3.9063 0.9766 65.92 KB
Bench 10 120.0 us 0.45 us 0.40 us 8,336.6686 4.0283 0.9766 67.07 KB
Bench 100 171.3 us 0.22 us 0.19 us 5,836.0763 4.6387 0.9766 78.03 KB
Bench 1000 1,314.9 us 5.81 us 5.15 us 760.5070 9.7656 1.9531 183.54 KB
Bench 10000 75,168.9 us 204.31 us 181.11 us 13.3034 - - 1338.61 KB
Bench 100000 7,070,723.0 us 71,374.12 us 59,600.63 us 0.1414 - - 12277.06 KB

After:

Method Count Mean Error StdDev Op/s Gen0 Gen1 Gen2 Allocated
Bench 1 116.2 us 2.18 us 1.94 us 8,602.9749 3.9063 0.9766 - 65.92 KB
Bench 10 118.5 us 1.08 us 0.91 us 8,437.0996 4.0283 0.9766 - 67.07 KB
Bench 100 165.1 us 1.35 us 1.13 us 6,055.4561 4.6387 0.9766 - 78.03 KB
Bench 1000 836.2 us 5.29 us 4.42 us 1,195.8540 10.7422 1.9531 - 183.54 KB
Bench 10000 28,742.7 us 65.45 us 51.10 us 34.7915 62.5000 31.2500 31.2500 1338.17 KB
Bench 100000 2,368,759.3 us 2,731.57 us 2,280.98 us 0.4222 - - - 12277.06 KB

That offers a significant boost of performance when the dependent entities are over 1000.

I'm a little confused about the Gen 0 which seems to give unreliable results. Maybe someone with more BenchmarkDotnet would have an explanation why it's sometimes -.

I tried using IEnumerable.Contains, I thought it would use a vectorized path but I learnt that it's not possible for reference search (dotnet/runtime#117178). After some type checks it falls on a slow path which is way slower (206 ops/s for Count=1000) than the manual loop. I believe it's because of the IEqualityComparer.Equals virtual calls.

@verdie-g verdie-g requested a review from a team as a code owner July 1, 2025 00:07
@AndriySvyryd AndriySvyryd enabled auto-merge (squash) July 2, 2025 18:58
@AndriySvyryd
Copy link
Member

Thanks for your contribution!

@AndriySvyryd AndriySvyryd merged commit 0a01abe into dotnet:main Jul 2, 2025
7 checks passed
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.

2 participants