4

I wrote the following to convert a byte array data to a string array hex containing 32 bytes as hex string per entry to write them to a file.

byte[] data = new byte[4*1024*1024];
string[] hex = data.Select((b) => b.ToString("X2")).ToArray();
hex = Enumerable.Range(0, data.Length / 32).Select((r) => String.Join(" ", hex.Skip(r * 32).Take(32))).ToArray();  // <= This line takes forever

The problem was that it took minutes(!) to finish although the resulting file was less than 20MB. So I tried to optimize it and came up with the following:

byte[] data = new byte[4*1024*1024];
string[] hex = new string[4*1024*1024/32];
for (var i = 0; i <= hex.Length - 1; i++)
{
    var sb = new System.Text.StringBuilder();
    sb.Append(data[i * 32].ToString("X2"));
    for (var k = 1; k <= 32 - 1; k++)
    {
        sb.Append(' ');
        sb.Append(data[i * 32 + k].ToString("X2"));
    }
    hex[i] = sb.ToString();
}

This version does the same but is several orders of magnitude faster (133 ms vs 8 minutes). My problem is that I don't really understand why the original version is so slow. I looked at the source of String.Join() and it looks pretty similar to my improved version. I like to use LINQ for these kind of thinks, because you can solve all kinds of problems pretty easily and I thought it was efficient in most cases because of it's lazy evaluation. So I would like to know what I am missing here to improve my future usage of LINQ.

As a side not I know that it could probably be written even faster but this is really not the point here, because the second version is fast enough for a function only used for debugging purposes.

Karsten
  • 1,814
  • 2
  • 17
  • 32
  • Could you not run a performance viewer to see where the time is being taken up? From a quick glance it may be the difference between string and string builder and I think in the first example you have a few more loops but without doing in-depth analysis its hard to say – Mark Davies Nov 20 '19 at 16:50
  • 2
    That Linq query is going to iterate over the `data` over and over because of the embedded `Skip().Take()`. Also I think you meant to use `hex` instead. An alternative would be to use the `Select` that includes the index and group on the index divided by 32 to get each chunk of 32, and then iterate over those results and use a string builder. – juharr Nov 20 '19 at 16:51
  • @MarkDavies At the moment I am doubting my understanding of LINQ and am looking for some kind of general answer what is going on here. Apart from that I did run some tests. I am pretty sure most of the time is spent in `String.Join()`, but I don't know why it is so slow in this case. This is important for me because I use `String.Join()` quite frequently and would like to know when to avoid it. – Karsten Nov 20 '19 at 16:58
  • 1
    My guess is strings being immutable, string.join() creates new instances of string. Time gets wasted in creating new objects and may be even cleaning it. It would be interesting to see how many objects are created in both cases. – AlwaysAProgrammer Nov 20 '19 at 17:04
  • 1
    Ah, right; you're using a StringBuilder in your second example. – Robert Harvey Nov 20 '19 at 17:05

2 Answers2

5

My problem is that I don't really understand why the original version is so slow.

It's this part:

hex.Skip(r * 32)

.Skip() has to walk the sequence. It doesn't get to jump straight to the correct index. In other words, for every 32 bytes in the array, you re-walk the entire array from the beginning until you get to the start of the current chunk. It's a Shlemiel the Painter situation.

You could potentially also make the original code faster by using an ArraySegment type, Array.Copy(), or Span<string>. You could also write your own linq-like "Chunk()" operator to return 32-byte sequences from an original IEnumerable, or use this very simple Segment() method:

public static IEnumerable<T> Segment<T>(this T[] original, int start, int length)
{
    length = start + length;
    while (start < length) 
        yield return original[start++];
}

which would change the original code to look like this:

byte[] data = new byte[4*1024*1024];
string[] hex = data.Select((b) => b.ToString("X2")).ToArray();
hex = Enumerable.Range(0, data.Length / 32).Select((r) => String.Join(" ", hex.Segment(r * 32,32))).ToArray();

and, for fun, using the Chunk() implementation I linked earlier:

byte[] data = new byte[4*1024*1024];
var hex = data.Select(b => b.ToString("X2"))
              .Chunk(32)
              .Select(c => string.Join(" ", c))
              .ToArray(); //only call ToArray() if you *really* need the array. Often the enumerable is enough.

Another fun option using String.Create()

byte[] data = new byte[4*1024*1024];
char[] hexChars = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
                    'A', 'B', 'C', 'D', 'E', 'F' };
var hex = data.Chunk(32)
      .Select(c => string.Create(95, c, (r, d) => {
          int i = 0;
          foreach(byte b in d)
          {
             r[i*3] = hexChars[((b & 0xf0) >> 4)];
             r[(i*3) + 1] = hexChars[(b & 0x0f)];
             if (i*3 < 92) r[(i*3) + 2] = ' ';
             i++;
         }
      }))
      .ToArray();

You should also look at this BitConverter.ToString() overload.

I'd love to see how each of these benchmark.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
  • You are right! I always thought LINQ was a little bit cleverer and treats arrays differently. But apparently it does not and treats it like any other `Enumerable`. – Karsten Nov 20 '19 at 17:09
  • In some cases, it can treat arrays differently. But not this one. – Joel Coehoorn Nov 20 '19 at 17:10
  • Sadly it does not do that in any case. I just looked at the source and the `SkipIterator` is as dumb as it can be... – Karsten Nov 20 '19 at 17:12
  • Skip() is dumb. Some others are smarter. – Joel Coehoorn Nov 20 '19 at 17:14
  • 2
    I ended up using `ArraySegment`. That way I can use the LINQ version with almost no changes and the performance is even a little bit better than my second version. – Karsten Nov 20 '19 at 17:23
2

The Take implementation of .NET Framework does not include any optimization for sources of type IList, so it becomes very slow when called repeatedly for large lists or arrays. The corresponding implementation of .NET Core includes these optimizations, so it performs quite decently (on a par with a manually coded loop).

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • In my case `Skip()` was the problem. In .NET Core `Skip()` has [this optimization](https://github.com/dotnet/corefx/blob/e99ec129cfd594d53f4390bf97d1d736cff6f860/src/System.Linq/src/System/Linq/Skip.SpeedOpt.cs#L32), too. In .NET Framework it does not. – Karsten Nov 20 '19 at 21:35
  • @Karsten you are right. I frequently confuse `Skip` with `Take`. :-) – Theodor Zoulias Nov 20 '19 at 21:52