Which piece of code is more performant?

Question

Hi folks,

I have some code i'm revewing, which is used to convert some text into an MD5 Hash. Works great. It's used to create an MD5Hhash for a gravatar avatar. Here it is :-

static MD5CryptoServiceProvider md5CryptoServiceProvider = null;

public static string ToMD5Hash(this string value)
{
    //creating only when needed
    if (md5CryptoServiceProvider == null)
    {
        md5CryptoServiceProvider = new MD5CryptoServiceProvider();
    }

    byte[] newdata = Encoding.Default.GetBytes(value);
    byte[] encrypted = md5CryptoServiceProvider.ComputeHash(newdata);
    return BitConverter.ToString(encrypted).Replace("-", "").ToLower();
}

Notice how we create a MD5CryptoServiceProvider the first time this method is called? (lets not worry about race-conditions here, for simplicity).

I was wondering, is it more computationally expensive if i change the lines that are used to create the provider, to this...

using(var md5CryptoServiceProvider = new MD5CryptoServiceProvider())
{
    ... snip snip snip ....
}

Now, how is this method used/consumed? Well, imagine it's the home page of StackOverflow -> for each post, generate an md5 hash of the user, so we can generate their gravatar url. So the view could call this method a few dozen times.

Without trying to waste too much time stressing over premature optimzation, etc ... which would be better?

Answer 1

I'd be more interested in the thread-safefy... MSDN doesn't (unless I missed it) say that MD5CryptoServiceProvider is thread-safe, so IMO the best option is to have one per call...

It doesn't matter how quickly you can get the wrong answer ;-p

What you probably don't want to do (to fix the thread-safety issue) is have a static instance and lock around it... that'll serialize all your crypto code, when it could run in parallel on different requests.

Answer 2

The existing code I would expect to be very slightly faster, as it saves reconstructing the MD5CryptoServiceProvider on every call, but I'd also expect the time to be dominated by the call to ComputeHash().

Answer 3

I personnally can't see why it would matter either way, think of the lashing of other code that has to run just to generate a page. Its six of one and half a dozen of the other, the savings either way will be tiny.

Answer 4

Test it and time it. The first is more performant, but it is probably not significant.

using System;
using System.Diagnostics;
using System.Security.Cryptography;
using System.Text;

namespace ConsoleApplication11
{
    class Program
    {
        static void Main(string[] args)
        {
            Stopwatch timer=new Stopwatch();
            int iterations = 100000;
            timer.Start();
            for (int i = 0; i < iterations; i++)
            {
                string s = "test" + i;
                string t=s.ToMd5Hash0();
            }
            timer.Stop();
            Console.WriteLine(timer.ElapsedTicks);

            timer.Reset();
            timer.Start();
            for (int i = 0; i < iterations; i++)
            {
                string s = "test" + i;
                string t = s.ToMd5Hash1();
            }
            timer.Stop();
            Console.WriteLine(timer.ElapsedTicks);

            Console.ReadKey();
        }
    }
    public static class Md5Factory
    {
        private static MD5CryptoServiceProvider md5CryptoServiceProvider;
        public static string ToMd5Hash0(this string value)
        {
            if (md5CryptoServiceProvider == null)
            {
                md5CryptoServiceProvider = new MD5CryptoServiceProvider();
            }
            byte[] newData = Encoding.Default.GetBytes(value);
            byte[] encrypted = md5CryptoServiceProvider.ComputeHash(newData);
            return BitConverter.ToString(encrypted).Replace("-", "").ToLower();
        }
        public static string ToMd5Hash1(this string value)
        {
            using (var provider = new MD5CryptoServiceProvider())
            {
                byte[] newData = Encoding.Default.GetBytes(value);
                byte[] encrypted = provider.ComputeHash(newData);
                return BitConverter.ToString(encrypted).Replace("-", "").ToLower();
            }
        }
    }
}