Ayende @ Rahien

Hi!
My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

, @ Q c

Posts: 18 | Comments: 87

filter by tags archive

Stupid smart code: Solution

time to read 3 min | 537 words

The reason that I said that this is very stupid code?

public static void WriteDataToRequest(HttpWebRequest req, string data)
{
    var byteCount = Encoding.UTF8.GetByteCount(data);
    req.ContentLength = byteCount;
    using (var dataStream = req.GetRequestStream())
    {
        if(byteCount <= 0x1000) // small size, just let the system allocate it
        {
            var bytes = Encoding.UTF8.GetBytes(data);
            dataStream.Write(bytes, 0, bytes.Length);
            dataStream.Flush();
            return;
        }

        var buffer = new byte[0x1000];
        var maxCharsThatCanFitInBuffer = buffer.Length / Encoding.UTF8.GetMaxByteCount(1);
        var charBuffer = new char[maxCharsThatCanFitInBuffer];
        int start = 0;
        var encoder = Encoding.UTF8.GetEncoder();
        while (start < data.Length)
        {
            var charCount = Math.Min(charBuffer.Length, data.Length - start);

            data.CopyTo(start, charBuffer, 0, charCount);
            var bytes = encoder.GetBytes(charBuffer, 0, charCount, buffer, 0, false);
            dataStream.Write(buffer, 0, bytes);
            start += charCount;
        }
        dataStream.Flush();
    }
}

Because all of this lovely code can be replaced with a simple:

public static void WriteDataToRequest(HttpWebRequest req, string data)
{
    req.ContentLength = Encoding.UTF8.GetByteCount(data);

    using (var dataStream = req.GetRequestStream())
    using(var writer = new StreamWriter(dataStream, Encoding.UTF8))
    {
        writer.Write(data);
        writer.Flush();
    }
}

And that is so much better.


Comments

Steinar

Much nicer indeed. But why the call writer.Flush()? Isn't that done in dispose?

Darren Cauthon

Ayende, if your example didn't have some sort of testing around it, I'd say it was just as stupid as the first.

João Angelo

With the new approach the UTF8 preamble will also be written to the request stream, this was not happening in the old code. Is this change on purpose?

Chris

@Steinar Flush() is called in Dispose() when certain conditions are met - in this case, I believe they are

Ayende Rahien

Joao, Yes, that is intentional. It is going over the wire, so it isn't actually meaningful from our point of view, but it is nice.

Ayende Rahien

Chris, There are stream and stream writer implementations that have different buffering strategies, and I rather avoid those sort of bugs

tobi

Ayende, all BCL streams and writers flush on dispose. Not doing that would be a horrible usability story.

I really hate the Flush method. It is almost never useful.

Chris

@Ayende, I was actually looking at Syste.IO.StreamWriter (.net 4.0) in dotPeek, but I guess that if you're using a different one (are you?) then Flush() could be mandatory

Chris

spelling correction, should be System.IO.StreamWriter

Ayende Rahien

Chris, Yes, that is the problem, you can't assume that. In general, that is not a mandatory behavior, and Flush ensures that it behaves properly

Jarek Kowalski

Out of curiosity, why not simply write to the stream?

public static void WriteDataToRequest(HttpWebRequest req, string data) { byte[] dataBytes = Encoding.UTF8.GetBytes(data);

req.ContentLength = dataBytes.Length;
using (var dataStream = req.GetRequestStream())
{
    dataStream.Write(dataBytes, 0, dataBytes.Length);
}

}

It saves the cost of double UTF-8 encoding at the expense of a potentially large allocation (btw. string already has a buffer of this size, so I don't know if it matters).

Ayende Rahien

Jarek, That is what we had in the beginning. The problem with that is that you have to allocate the entire buffer for that. That means that if you have a 500Kb string, you have a 500 Kb byte array, and that is a pure waste.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

  1. Buffer allocation strategies: A possible solution - about one day from now
  2. Buffer allocation strategies: Explaining the solution - 3 days from now
  3. Buffer allocation strategies: Bad usage patterns - 4 days from now
  4. The useless text book algorithms - 5 days from now
  5. Find the bug: The concurrent memory buster - 6 days from now

There are posts all the way to Sep 11, 2015

RECENT SERIES

  1. Find the bug (5):
    20 Apr 2011 - Why do I get a Null Reference Exception?
  2. Production postmortem (10):
    03 Sep 2015 - The industry at large
  3. What is new in RavenDB 3.5 (7):
    12 Aug 2015 - Monitoring support
  4. Career planning (6):
    24 Jul 2015 - The immortal choices aren't
View all series

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats