1

I have a program that will write a series of files in a loop. The filename is constructed using a parameter from an object supplied to the method.

ANTS Performance Profiler says this is dog slow and I'm not sure why:

public string CreateFilename(MyObject obj)
{
    return "sometext-" + obj.Name + ".txt";
}

Is there a more performant way of doing this? The method is hit thousands of times and I don't know of a good way outside of having a discrete method for this purpose since the input objects are out of my control and regularly change.

joelc
  • 2,687
  • 5
  • 40
  • 60
  • 1
    Benchmark it with `String.Format()` function, I would recon that its slightly faster. But tbh, the cost of concatenation is not very high, i would guess this is unnecessary optimization? – Jite Dec 21 '16 at 20:48
  • how often is this being called per `obj`? – Daniel A. White Dec 21 '16 at 20:49
  • 1
    use `StringBuilder`. obviously `StringBuilder` should be some kind of private field to initialize it only once. but use it as much as you want. – M.kazem Akhgary Dec 21 '16 at 20:49
  • No, there's no faster way. Calling `String.Format` will do essentially the same thing. – Jim Mischel Dec 21 '16 at 20:49
  • You could use interpolation but I am not sure if it is faster or not: `return $"sometext-{obj.Name}.txt";` – Igor Dec 21 '16 at 20:50
  • 2
    @M.kazemAkhgary string.format uses a string builder under the covers. – Daniel A. White Dec 21 '16 at 20:50
  • `StringBuilder` is great for dynamic strings but for a simple concatenation like this I doubt it make this line more efficient. – Igor Dec 21 '16 at 20:51
  • 1
    You could also try to inline it with `[MethodImpl(MethodImplOptions.AggressiveInlining)]` and see if that helps a bit. This method feels like a typical thing to inline when its possible! – Jite Dec 21 '16 at 20:54
  • Also its normal that performance profilers list these kind of methods at top. when a method is _hit thousands of times_ what else do you expect? it doesn't necessarily mean the method itself is slow. – M.kazem Akhgary Dec 21 '16 at 20:55
  • 1
    Are you sure its the string concat and not the .Name method that is slow? Also, how slow is dog-slow? Sometimes what you are looking at in the profiler can be a little misleading. are you looking at clock time or processor time? – Jason Hernandez Dec 21 '16 at 20:58
  • 3
    `String.Format` and `StringBuilder` will be significantly slower for this. Don't know why anyone is recommending either of those. – Cory Nelson Dec 21 '16 at 20:59
  • 1
    Seems surprising that this will be a significant time relative to that of writing the file. – Martin Smith Dec 21 '16 at 21:12
  • 1
    Give us some context. How many times is that code being called, and how many nanoseconds per call are you spending in it? – Eric Lippert Dec 21 '16 at 21:27
  • Hi all, it's being called hundreds of time per second. Each call is quite fast, but it's being hit so often that it's taking a measurable percentage of time compared to the write of the file (they are very small files, like 4KB). I'll try the aggressive inlining suggestion above, too. Thank you! – joelc Dec 22 '16 at 00:47
  • Hundreds of times per second is basically never. Ask again when you're hitting hundreds of thousands. – Veedrac Dec 22 '16 at 09:16
  • If 100 times per second is causing performance issues, try throwing hardware at the problem. My machine can run that function millions of times per second. – Brian Dec 22 '16 at 14:30
  • @Veedrac sorry my problems are not large enough for you. – joelc Dec 22 '16 at 16:45
  • @Brian the operation is latency sensitive, which is why I asked. Thanks – joelc Dec 22 '16 at 16:45
  • 2
    @joelc There's no point optimizing 0.01% of your bottleneck. I'm not trying to be funny here, it's just that you're wasting your time. The bottleneck is elsewhere. – Veedrac Dec 22 '16 at 18:04

2 Answers2

8

The compiler will optimize your two concats into one call to:

String.Concat("sometext-", obj.Name, ".txt")

There is no faster way to do this.

Cory Nelson
  • 29,236
  • 5
  • 72
  • 110
2

If you instead compute the filename within the class itself, it will run much faster, in exchange for decreased performance when modifying the object. Mind you, I'd be very concerned if computing a filename was a bottleneck; writing to the file is way slower than coming up with its name.

See code samples below. When I benchmarked them with optimizations on (in LINQPad 5), Test2 ran about 15x faster than Test1. Among other things, Test2 doesn't constantly generate/discard tiny string objects.

void Main()
{
    Test1();
    Test1();
    Test1();
    Test2();
    Test2();
    Test2();
}

void Test1()
{
    System.Diagnostics.Stopwatch sw = new Stopwatch();
    MyObject1 mo = new MyObject1 { Name = "MyName" };
    sw.Start();
    long x = 0;
    for (int i = 0; i < 10000000; ++i)
    {

        x += CreateFileName(mo).Length;
    }
    Console.WriteLine(x); //Sanity Check, prevent clever compiler optimizations
    sw.ElapsedMilliseconds.Dump("Test1");
}

public string CreateFileName(MyObject1 obj)
{
    return "sometext-" + obj.Name + ".txt";
}

void Test2()
{
    System.Diagnostics.Stopwatch sw = new Stopwatch();
    MyObject2 mo = new MyObject2 { Name = "MyName" };
    sw.Start();
    long x = 0;
    for (int i = 0; i < 10000000; ++i)
    {
        x += mo.FileName.Length;
    }
    Console.WriteLine(x); //Sanity Check, prevent clever compiler optimizations
    sw.ElapsedMilliseconds.Dump("Test2");
}



public class MyObject1
{
    public string Name;
}

public class MyObject2
{
    public string FileName { get; private set;}
    private string _name;
    public string Name
    {
        get 
        {
            return _name; 
        }
        set 
        {
            _name=value;
            FileName = "sometext-" + _name + ".txt";
        }
    }
}

I also tested adding memoization to CreateFileName, but it barely improved performance over Test1, and it couldn't possibly beat out Test2, since it performs the equivalent steps with additional overhead for hash lookups.

Brian
  • 25,523
  • 18
  • 82
  • 173