Loading and converting data from CSV

Question

My application imports a CSV file and parses each row, turning it into an object in my code.

The speed for a 500,000 row x 68 column takes 2+ minutes to read. I've narrowed down the bottleneck to the part where the strings are being converted into their respective types using the converter I created. I need some pointers on how I can optimize my code.

This is the code I use to load:

public static void Load(object target, string[] fields, bool supressErrors)
{
    Type targetType = target.GetType();
    PropertyInfo[] properties = targetType.GetProperties();

    foreach (PropertyInfo property in properties)
    {
        object[] attributes = property.GetCustomAttributes(typeof(CSVPositionAttribute), false);

        if (attributes.Length > 0)
        {
            CSVPositionAttribute positionAttr = (CSVPositionAttribute)attributes[0];

            int position = positionAttr.Position;
            try
            {
                object data = fields[position];

                if (positionAttr.DataTransform != string.Empty)
                {
                    MethodInfo method = targetType.GetMethod(positionAttr.DataTransform);
                    data = method.Invoke(target, new object[] { data });
                }
                var prop = property.PropertyType;

                //bottleneck here:
                if (prop == typeof(string))
                {
                    property.SetValue(target, CheckAndConvert.CheckString(data), null);
                }
                else if (prop == typeof(int))
                {
                    property.SetValue(target, CheckAndConvert.CheckInt(data.ToString()), null);
                }
                else if (prop == typeof(double))
                {
                    property.SetValue(target, CheckAndConvert.CheckDouble(data.ToString()), null);
                }
                else if (prop == typeof(long))
                {
                    property.SetValue(target, CheckAndConvert.CheckLong(data.ToString()), null);
                }
                else if (prop == typeof(bool))
                {
                    property.SetValue(target, CheckAndConvert.CheckBool(data.ToString()), null);
                }

            }
            catch(Exception e)
            {
                if (!supressErrors)
                    throw;
            }
        }
    }
}

This is part of the code I use to convert each cell:

public class CheckAndConvert
{
    public static int CheckInt(string input)
    {
        int convertedInput = 0;
        double convertedInput2 = 0;

        if (int.TryParse(input, out convertedInput))
        {
            return convertedInput;
        }
        else if (double.TryParse(input, out convertedInput2))
        {
            return (int)convertedInput2;
        }
        else
        {
            return 0;
        }
    }
    public static double CheckDouble(string input)
    {
        double convertedInput = 0;
        int convertedInput2 = 0;
        if (double.TryParse(input, out convertedInput))
        {
            return convertedInput;
        }
        else if (int.TryParse(input, out convertedInput2))
        {
            return convertedInput2;
        }
        else
        {
            return 0;
        }
    }
    public static bool CheckBool(string input)
    {
        bool convertedInput = true;
        if (bool.TryParse(input, out convertedInput))
        {
            return convertedInput;
        }
        else
        {
            return true;
        }
    }
    public static string CheckString(object input)
    {
        if (input == null)
        {
            return "";
        }
        else
        {
            return input.ToString();
        }
    }

}

Answer 1

There's a few ways you could improve your CheckAndConvert class and functions.

Design Suggestions

• Fluency - It's not a requirement by any means but fluency helps readability. CheckAndConvert.CheckInt is almost a sentence, perhaps something shorter Parse.Int or Parse.Bool.
• Naming Simplicity - On the lines of the previous suggestion, your class/function names imply that it's doing more than simply parsing a given value. You're checking if the parse succeeds but then returning a default value if it fails. The user of this function needs not to know about the check part at all. And the word check is repeated (class and function name). Programmers don't typically like redundancy.
• Naming Accuracy - CheckInt and CheckDouble they check each other if their primary data type TryParse fails. In the case of CheckDouble I don't think it's ever going to be true for the second if-statement. In the case of CheckInt since int.TryParse can fail on decimals that you want to be integers I would just always use double.TryParse with a cast. (Remember: Cast truncates and Convert.ToInt32 rounds).

I would be surprised if any of the above suggestions other than maybe the removal of int.TryParse would actually improve performance but it would still make for some cleaner simpler code.

Performance

If you've narrowed down that the parsing of values into their respective properties is the time consumer research alternative ways to parse numbers (they're ugly but claim they're faster) additionally this might be helpful for possibly speeding up your SetValue calls (be warned it's a long read).

Link Information

• The parse numbers link decribes parsing the string yourself as .Parse uses CultureInfo and stuff that can slows it down.
• The SetValue link describes research into using delegates for method calls in reflection and shows some promising performance boosts that I've used myself.

Answer 2

The code, for a 'target' type, runs reflection code, for each row.

For a given 'target' type, I would generate a list of functions only once, and invoke them instead, multiple times, for each row.

See my response, to another question, where I use a similar technique which improved performance. https://codereview.stackexchange.com/a/87810/35554

Answer 3

• you are passing string[] fields to the method and later you do object data = fields[position]; and use it like data.ToString(). That is free of sense. Declare data as string and remove the calls to ´ToString()`.

• always use braces although they might be optional like for single line if statements. Using them will make your code less error prone.

• In the helper methods for instance in CheckBool

  public static bool CheckBool(string input)
  {
      bool convertedInput = true;
      if (bool.TryParse(input, out convertedInput))
      {
          return convertedInput;
      }
      else
      {
          return true;
      }
  }  
  

  there is no need to initialize convertedInput because it is used as an out parameter and therefor its value will overwritten.

• speaking about the helper methods, all of their names are wrong. They don't Check but Convert.

Answer 4

Here is my few suggestions.

1. Declare heavy objects outside of the foreach loop. In a for loop, it creates and destroys the objects. If you create outside of the class it saves some CPU cycles. For example:

object[] attributes;
CSVPositionAttribute positionAttr;

2. use the string.IsNullOrEmpty instead of != operator.

if (string.IsNullOrEmpty(positionAttr.DataTransform)==false)

3. If you can use the traditional for loop that would be good option. The old style for loop is faster than foreach.

Answer 5

Fast reflection

As hocho said, if you want fast reflection you have to setup the parsing once for each type.

Then if you wan't to improve the performance further you have to get away from calls to Invoke, SetValue and so on.

To get an idea of how to do that you can read MAKING REFLECTION FLY AND EXPLORING DELEGATES by Jon Skeet.

For further inspiration you can look at Protocol Buffers for .Net and Jil.

---

How does it look in practice? First a row class and your attribute:

public class CSVPositionAttribute : Attribute
{
    public int Position { get; }
    public string DataTransform { get; set;}

    public CSVPositionAttribute(int position)
    {
        Position = position;
    }
}

public class Hest
{
    [CSVPosition(0)]
    public string Name { get; set; }
    [CSVPosition(1, DataTransform = nameof(ParseSize))]
    public int Size { get; set; }
    [CSVPosition(2, DataTransform = nameof(Convert))]
    public string Appearance { get; set; }

    public override string ToString()
    {
        return Name;
    }

    public string Convert(string data)
    {
        return new string(data.Reverse().ToArray());
    }

    public int ParseSize(string size)
    {
        return System.Convert.ToInt32(size);
    }
}

Then you'll some helpers like the ones from the blog post to create open delegates:

public static class ReflectionHelper
{
    public static Func<T, object, object> CreateFunctionDelegate<T>(MethodInfo method) where T : class
    {
        // First fetch the generic form
        MethodInfo genericHelper = typeof(ReflectionHelper).GetMethod(nameof(CreateFunctionHelper),
            BindingFlags.Static | BindingFlags.NonPublic);

        // Now supply the type arguments
        MethodInfo constructedHelper = genericHelper.MakeGenericMethod
            (typeof(T), method.GetParameters()[0].ParameterType, method.ReturnType);

        // Now call it. The null argument is because it’s a static method.
        object ret = constructedHelper.Invoke(null, new object[] { method });

        // Cast the result to the right kind of delegate and return it
        return (Func<T, object, object>)ret;
    }

    private static Func<TTarget, object, object> CreateFunctionHelper<TTarget, TParam, TReturn>(MethodInfo method)
        where TTarget : class
    {
        // Convert the slow MethodInfo into a fast, strongly typed, open delegate
        Func<TTarget, TParam, TReturn> func = (Func<TTarget, TParam, TReturn>)Delegate.CreateDelegate
            (typeof(Func<TTarget, TParam, TReturn>), method);

        // Now create a more weakly typed delegate which will call the strongly typed one
        Func<TTarget, object, object> ret = (TTarget target, object param) => func(target, (TParam)param);
        return ret;
    }

    public static Action<T, object> CreateActionDelegate<T>(MethodInfo method) where T : class
    {
        // First fetch the generic form
        MethodInfo genericHelper = typeof(ReflectionHelper).GetMethod(nameof(CreateActionHelper),
            BindingFlags.Static | BindingFlags.NonPublic);

        // Now supply the type arguments
        MethodInfo constructedHelper = genericHelper.MakeGenericMethod
            (typeof(T), method.GetParameters()[0].ParameterType);

        // Now call it. The null argument is because it’s a static method.
        object ret = constructedHelper.Invoke(null, new object[] { method });

        // Cast the result to the right kind of delegate and return it
        return (Action<T, object>)ret;
    }

    private static Action<TTarget, object> CreateActionHelper<TTarget, TParam>(MethodInfo method)
        where TTarget : class
    {
        // Convert the slow MethodInfo into a fast, strongly typed, open delegate
        var action = (Action<TTarget, TParam>)Delegate.CreateDelegate(typeof(Action<TTarget, TParam>), method);

        // Now create a more weakly typed delegate which will call the strongly typed one
        Action<TTarget, object> ret = (TTarget target, object param) => action(target, (TParam)param);
        return ret;
    }
}

And then a generic class construct and cache the parsing delegate:

private static class RowParser<T>
    where T : class
{
    private static Action<T, string[]> _parse = InitializeParse();

    private static Action<T, string[]> InitializeParse()
    {
        var targetType = typeof(T);
        var propertyAssignments = targetType.GetProperties()
            .Select(p => new
            {
                Property = p,
                CSVPosition = (CSVPositionAttribute)p.GetCustomAttributes(typeof(CSVPositionAttribute), false)
                               .SingleOrDefault()
            })
            .Where(x => x.CSVPosition != null)
            .Select(x => CreatePropertyParser(x.Property, x.CSVPosition.DataTransform, x.CSVPosition.Position))
            .ToList();

        var target = Expression.Parameter(typeof(T), "target");
        var fields = Expression.Parameter(typeof(string[]), "fields");

        var callAllMethod = typeof(RowParser<T>).GetMethod(nameof(CallPropertyAssigners),
            BindingFlags.Static | BindingFlags.NonPublic);

        var callAllExpression = Expression.Call(callAllMethod,
            Expression.Constant(propertyAssignments),
            target,
            fields);

        var lambda = Expression.Lambda<Action<T, string[]>>(callAllExpression, target, fields);

        return lambda.Compile();
    }

    private static void CallPropertyAssigners(List<Action<T, string[]>> propertyAssigners, T target, string[] fields)
    {
        foreach (var assigner in propertyAssigners)
        {
            assigner(target, fields);
        }
    }

    private static Action<T, string[]> CreatePropertyParser(PropertyInfo property, string converterName, int position)
    {
        var propertySetter = ReflectionHelper.CreateActionDelegate<T>(property.SetMethod);

        var target = Expression.Parameter(typeof(T), "target");
        var fields = Expression.Parameter(typeof(string[]), "fields");

        Expression data = Expression.ArrayAccess(fields, Expression.Constant(position));

        if (!string.IsNullOrWhiteSpace(converterName))
        {
            var converterAction = ReflectionHelper.CreateFunctionDelegate<T>(typeof(T).GetMethod(converterName));
            data = Expression.Call(Expression.Constant(converterAction.Target), converterAction.Method, target, data);
        }

        var callSetter = Expression.Call(Expression.Constant(propertySetter.Target), propertySetter.Method, target, data);

        var lambda = Expression.Lambda<Action<T, string[]>>(callSetter, target, fields);

        return lambda.Compile();
    }

    public static void Parse(T target, string[] fields)
    {
        _parse(target, fields);
    }
}

Then you can call:

var hest = new Hest();
var fields = new[]{ "Bent", "14", "Grim" };
for(int i = 0; i < 500000; i++)
{
    RowParser<Hest>.Parse(hest, fields);
}

This snippet takes 300 milliseconds to run on my laptop. It does however require that you have the type at compile time but it's easy to create another helper to make this work for any object at runtime.