patterncsharpMinor
performance on this substring search
Viewed 0 times
thissubstringperformancesearch
Problem
I have the code below that gets hit several hundred times per second. I'm wondering what I can do to improve performance. It seems that there should be some way to build up a substring index. Is there some existing examples of that or an explanation on how to do this?
``
``
using System;
using System.Collections.Generic;
using System.Linq;
using Asi.Server.Interfaces.History;
namespace Asi.Server.History.AssetHistory
{
public class TelemetrySubscriptionFilter: ITelemetryLogFilter, ITelemetryLogAutosubscriber
{
private readonly List _shouldNotLogs = new List
{
"Local Position Service",
"Global Position Service",
"PositionGroup.",
"DKS/"
};
public bool ShouldNotLog(string telemetryName)
{
float period;
if (ShouldAutoSubscribe(telemetryName, out period))
return false;
// telemetryName is longer than our filter texts
return _shouldNotLogs.Any(s => telemetryName.IndexOf(s, StringComparison.Ordinal) >= 0);
}
private readonly List _shouldAutoSubscribe = new List
{
"Manual Mode",
"Autonomous Driven",
"Battery Voltage",
"RPM",
"Engine On",
"Parking Brake",
"Gear",
"Stopping Distance",
"Ready For Motion",
"GPS Correction Sent",
"Setpoint",
"Dead Reckon",
"VCU uC",
"Feedback",
"Off Path",
"RMS",
"Yaw Rate",
"Stop Enabled",
"Arbiter",
"Velocity Error",
"Processor"
};
public bool ShouldAutoSubscribe(string telemetryName, out float period)
{
period = 0.375f; // chosen somewhat arbitrarily
return _shouldAutoSubscribe.Any(s => telemetryName.IndexOf(s, StringComparison.Ordinal) >= 0);
}
}
}
Solution
As with every performance question: run your code through a profiler
That said, your
As a result, checking _shouldAutoSubscribe only adds extra work. The only times you perform fewer operations is if the input name matches one of the first three elements in _shouldAutoSubscribe. It's a wash at four. Anything else is extra work.
This leaves us with the following:
Next, for readability, I would instead use
If possible, though, an even better way of doing it would be to extract your telemetry names to some numerical ID and use enums or ints. Number comparison will be far quicker than string comparison. However, you state that your input names are superstrings of those appearing in your lists, so I don't know if that is feasible.
That said, your
ShouldNotLog method can be simplified. There is not much reason to check your auto subscribe list prior to checking your suppress list:- _shouldNotLogs is a shorter list
- presumably, none of the strings in _shouldNotLogs should exist in _shouldAutoSubscribe
As a result, checking _shouldAutoSubscribe only adds extra work. The only times you perform fewer operations is if the input name matches one of the first three elements in _shouldAutoSubscribe. It's a wash at four. Anything else is extra work.
This leaves us with the following:
public bool ShouldNotLog(string telemetryName)
{
return _shouldNotLogs.Any(s => telemetryName.IndexOf(s, StringComparison.Ordinal) >= 0);
}Next, for readability, I would instead use
string.Contains. If you check the .NET source, string.Contains calls string.IndexOf internally with an Ordinal check, so it is effectively the same thing, but it declares your intent far better:public bool ShouldNotLog(string telemetryName)
{
return _shouldNotLogs.Any(s => telemetryName.Contains(s));
}If possible, though, an even better way of doing it would be to extract your telemetry names to some numerical ID and use enums or ints. Number comparison will be far quicker than string comparison. However, you state that your input names are superstrings of those appearing in your lists, so I don't know if that is feasible.
Code Snippets
public bool ShouldNotLog(string telemetryName)
{
return _shouldNotLogs.Any(s => telemetryName.IndexOf(s, StringComparison.Ordinal) >= 0);
}public bool ShouldNotLog(string telemetryName)
{
return _shouldNotLogs.Any(s => telemetryName.Contains(s));
}Context
StackExchange Code Review Q#55518, answer score: 3
Revisions (0)
No revisions yet.