Skip to content
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.

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

Improve performance of ConsolidatorScanPriority comparison #8452

Conversation

starteleport
Copy link
Contributor

Description

This introduces a new Comparer and supplies it to the PrioorityQueue constructor. This yields 5-10 percent improvement in data points per second for all Lean benchmarks on my machine.

Related Issue

See issue #8451

Motivation and Context

The ConsolidatorScanPriority type implements IComparable and is used as the priority in the SubscriptionManager's priority queue. Due to the way comparison is handled for the case when PriorityQueue is not supplied with IComparer instance explicitly, an intermediate ObjectComparer is used for the comparison that eventually delegates comparison to ConsolidatorScanPriority.CompareTo.

Requires Documentation Change

No

How Has This Been Tested?

Lean benchmarks

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor (non-breaking change which improves implementation)
  • Performance (non-breaking change which improves performance. Please add associated performance test and results)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Non-functional change (xml comments/documentation/etc)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

@starteleport starteleport changed the title Improve the performance of ConsolidatorScanPriority comparison Improve performance of ConsolidatorScanPriority comparison Dec 9, 2024
@Martin-Molinero
Copy link
Member

Hey @starteleport! Thank you for the contribution. Could you please add some simple test/benchmark proof we can run to assert the improvement? A reference/documentation proof would be great too 🙏
A quick review, I see priority queue seems to be creating a single comparer instance.
image

@starteleport
Copy link
Contributor Author

Sure!

Here are the benchmark results:

|                             Method |     Mean |    Error |   StdDev |   Gen0 | Allocated |
|----------------------------------- |---------:|---------:|---------:|-------:|----------:|
|                           Baseline | 19.77 us | 0.321 us | 0.301 us | 0.1831 |   1.63 KB |
|       ComparerDelegatingToBaseline | 13.02 us | 0.272 us | 0.731 us | 0.1984 |   1.63 KB |
| SealedComparerDelegatingToBaseline | 12.68 us | 0.250 us | 0.267 us | 0.1984 |   1.63 KB |
|                   ExplicitComparer | 12.09 us | 0.176 us | 0.164 us | 0.1984 |   1.63 KB |
Benchmark code
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Diagnosers;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;

var summary = BenchmarkRunner.Run<Bench>();

[Config(typeof(BenchmarkConfig))]
public class Bench
{
    private (ConsolidatorScanPriority, ConsolidatorScanPriority)[] _priorities;

    [GlobalSetup]
    public void Setup()
    {
        _priorities = Enumerable.Range(1, 100)
            .Select(_ => new ConsolidatorScanPriority(DateTime.UtcNow, Random.Shared.NextInt64(1000)))
            .Select(c => (c, c))
            .ToArray();
    }

    [Benchmark]
    public long Baseline()
    {
        long count = 0;
        var queue = new PriorityQueue<ConsolidatorScanPriority, ConsolidatorScanPriority>(_priorities);

        while (queue.TryDequeue(out var element, out var priority))
        {
            count += element.Id;
        }

        return count;
    }

    [Benchmark]
    public long ComparerDelegatingToBaseline()
    {
        long count = 0;
        var queue = new PriorityQueue<ConsolidatorScanPriority, ConsolidatorScanPriority>(
            _priorities,
            ConsolidatorScanPriority.DelegatingComparer);

        while (queue.TryDequeue(out var element, out var priority))
        {
            count += element.Id;
        }

        return count;
    }

    [Benchmark]
    public long SealedComparerDelegatingToBaseline()
    {
        long count = 0;
        var queue = new PriorityQueue<ConsolidatorScanPriority, ConsolidatorScanPriority>(
            _priorities,
            ConsolidatorScanPriority.DelegatingSealedComparer);

        while (queue.TryDequeue(out var element, out var priority))
        {
            count += element.Id;
        }

        return count;
    }

    [Benchmark]
    public long ExplicitComparer()
    {
        long count = 0;
        var queue = new PriorityQueue<ConsolidatorScanPriority, ConsolidatorScanPriority>(
            _priorities,
            ConsolidatorScanPriority.ExplicitComparer);

        while (queue.TryDequeue(out var element, out var priority))
        {
            count += element.Id;
        }

        return count;
    }
}

public class BenchmarkConfig : ManualConfig
{
    public BenchmarkConfig()
    {
        // Configure your benchmarks, see for more details: https://benchmarkdotnet.org/articles/configs/configs.html.
        AddJob(Job.Default);
        //AddLogger(ConsoleLogger.Ascii);
        //AddExporter(MarkdownExporter.GitHub);
        AddDiagnoser(MemoryDiagnoser.Default);
        //Add(TargetMethodColumn.Method, StatisticColumn.Max);
        //Add(RPlotExporter.Default, CsvExporter.Default);
        //Add(EnvironmentAnalyser.Default);
    }
}

internal class ConsolidatorScanPriority : IComparable
{
    /// <summary>
    /// The next utc scan time
    /// </summary>
    public DateTime UtcScanTime { get; }

    private sealed class UtcScanTimeIdRelationalComparer : IComparer<ConsolidatorScanPriority>
    {
        public int Compare(ConsolidatorScanPriority? x, ConsolidatorScanPriority? y)
        {
            if (ReferenceEquals(x, y)) return 0;
            if (y is null) return 1;
            if (x is null) return -1;
            var utcScanTimeComparison = x.UtcScanTime.CompareTo(y.UtcScanTime);
            if (utcScanTimeComparison != 0) return utcScanTimeComparison;
            return x.Id.CompareTo(y.Id);
        }
    }

    public static IComparer<ConsolidatorScanPriority> ExplicitComparer { get; } =
        new UtcScanTimeIdRelationalComparer();

    internal class ConsolidatorScanPriorityComparer : IComparer<ConsolidatorScanPriority>
    {
        public int Compare(ConsolidatorScanPriority? x, ConsolidatorScanPriority? y)
        {
            return x?.CompareTo(y!) ?? -1;
        }
    }

    internal sealed class ConsolidatorScanPrioritySealedComparer : IComparer<ConsolidatorScanPriority>
    {
        public int Compare(ConsolidatorScanPriority? x, ConsolidatorScanPriority? y)
        {
            return x?.CompareTo(y!) ?? -1;
        }
    }

    public static IComparer<ConsolidatorScanPriority> DelegatingComparer { get; } =
        new ConsolidatorScanPriorityComparer();

    public static IComparer<ConsolidatorScanPriority> DelegatingSealedComparer { get; } =
        new ConsolidatorScanPrioritySealedComparer();

    /// <summary>
    /// Unique Id of the associated consolidator
    /// </summary>
    public long Id { get; }

    public ConsolidatorScanPriority(DateTime utcScanTime, long id)
    {
        Id = id;
        UtcScanTime = utcScanTime;
    }

    public int CompareTo(object obj)
    {
        if (obj == null) return 1;

        var other = (ConsolidatorScanPriority)obj;
        var result = UtcScanTime.CompareTo(other.UtcScanTime);
        if (result == 0)
        {
            // if they are the same let's compare Ids too
            return Id.CompareTo(other.Id);
        }

        return result;
    }
}

The whole point of this change is to avoid using ObjectComparer that gets created via InitializeComparer -> Comparer<TPriority>.Default -> ComparerHelpers.CreateDefaultComparer(typeof(T)), which in turn delegates to System.Collections.Comparer.Default.Compare that has extra type checks.

If this queue wasn't used in a hot spot, this optimisation wouldn't make much sense, but I believe it would be beneficial to grab this low hanging fruit.

Copy link
Member

@Martin-Molinero Martin-Molinero left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@Martin-Molinero Martin-Molinero merged commit 7445f7e into QuantConnect:master Dec 11, 2024
5 of 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