Skip to content

Conversation

@jpdillingham
Copy link
Member

Long story short, I discovered the CPU utilization rose substantially for each entry in the list of incoming search request filters, as did garbage collection pressure. I finally tracked it down to inefficiencies of IEnumerable handling; to iterate over one (such as Any() in this example) a whole state machine is created, and this can't be optimized out because of the dynamic nature.

I switched the type of collection for the compiled filter regular expressions from IEnumerable to Regex[] and the computation time for filtering dropped from ~45ms for a list of 30 filters to under 1ms (not sure how many ns but negligible).

image

Here we see, from left to right:

  • 2 search filters
  • Distributed network disabled (trying to find the source)
  • 15 search filters
  • 0 search filters

I'm positive that I've identified the culprit and have verified locally the benefits to this change. I'm expecting performance with just about any number of filters to be the same as with zero filters once this change is deployed.

@jpdillingham jpdillingham merged commit 2bb9297 into master Aug 4, 2025
4 checks passed
@jpdillingham jpdillingham deleted the regex branch August 4, 2025 23:54
@2048khz-gachi-rmx
Copy link

2048khz-gachi-rmx commented Aug 9, 2025

I'm pretty sure the real performance culprit comes from compiling new Regex instances over and over on each iteration, not from IEnumerable inefficiencies (i assume you meant that?). Compiled Regex'es are (usually) faster than interpreted, but take a while to actually compile, so instantiations should be done once (like on startup).

using System;
using System.Linq;

class TestClass
{
    public TestClass(int idx)
        => Console.WriteLine($"Instantiated new class: {idx}");
}

class Program
{
    static void Main(string[] args)
    {
        var range = Enumerable.Range(0, 10).Select(x => new TestClass(x));
        range.Any(_ => false);
        range.Any(_ => false);
        range.Any(_ => false);
    }
}

Good job on spotting the issue though; i was beginning to have concerns regarding slskd's CPU usage with barely any activity

@jpdillingham
Copy link
Member Author

These have always been compiled once when the app starts (or when options change):

var regexOptions = RegexOptions.Compiled;
if (!Flags.CaseSensitiveRegEx)
{
regexOptions |= RegexOptions.IgnoreCase;
}
CompiledSearchRequestFilters = OptionsAtStartup.Filters.Search.Request.Select(f => new Regex(f, regexOptions)).ToArray();

The only real difference was the .ToArray(). I think there may be a nuanced issue with using IEnumerable<Regex> for the property type but I didn't test changing just that.

Garbage collection pressure increased along with CPU, which makes me think there's some sort of synchronous collection going on along with the state machine lifecycle under the hood.

@2048khz-gachi-rmx
Copy link

2048khz-gachi-rmx commented Aug 9, 2025

The only real difference was the .ToArray().

IEnumerable produced by LINQ has deferred evaluation; it won't actually execute the expression until something forces it to materialize (.Any(), .ToList(), etc.). So, because CompiledSearchResponseFilters was storing an unmaterialized IEnumerable, each time something iterated over it (like calling .Any()), it was creating its' contents anew. And each time it did that, it was applying the .Select(f => new Regex(f, regexOptions)), thus recompiling all the regexes, which is slow

Just for demonstration purposes, you can try undoing the changes and then also get rid of the .Compiled flag. It should be faster, since it's not recompiling Regex anymore.

@jpdillingham
Copy link
Member Author

That's right, I forgot about that. I was just looking for similar optimizations in Soulseek.NET and was seeing .ToList() everywhere inside of constructors, so I knew this at one point :)

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.

3 participants