need help to understand why the performance increases in my code

Question

5.00/5 (1 vote)

See more:

I have some code that DOESN'T error so this is not a question of what is going wrong. But I am failing to work out why the speed of some code has increased significantly by changing some code structure.

Also apologies as this is a long worded question, but I am trying to understand why there is such a performance increase.

C#

public static void ToList<X, T>(this IEnumerable<T> Source, IList<X> Destintation) where X : new()
{
    PropertyDescriptorCollection SourceProperties = TypeDescriptor.GetProperties(typeof(T));
    PropertyDescriptorCollection DestinationProperties = TypeDescriptor.GetProperties(typeof(X));
    IDictionary<PropertyDescriptor, PropertyDescriptor> Map = new Dictionary<PropertyDescriptor, PropertyDescriptor>();

    //create a map using a dictionary
    foreach (PropertyDescriptor src in SourceProperties)
    {
        PropertyDescriptor dest = DestinationProperties.Find(src.Name, true);
        if (dest != null)
        {
            if (dest.PropertyType == src.PropertyType)
            {
                //add to dictionary
                Map.Add(src, dest);
            }
            else
            {
                //test for nullable types
                Type SourceNullable = Nullable.GetUnderlyingType(src.PropertyType);
                Type DestNullable = Nullable.GetUnderlyingType(dest.PropertyType);

                if (dest.PropertyType.Equals(SourceNullable) == true || src.PropertyType.Equals(DestNullable) == true)
                {
                    Map.Add(src, dest);
                }
            }
        }
    }

    foreach (T item in Source)
    {
        object record = Activator.CreateInstance(typeof(X));

        foreach (KeyValuePair<PropertyDescriptor, PropertyDescriptor> mapItem in Map)
        {
            PropertyDescriptor s = mapItem.Key;
            PropertyDescriptor d = mapItem.Value;

            d.SetValue(record, s.GetValue(item));
        }

        Destintation.Add((X)record);
    }
}

for the above code I use it to convert 1,000,000 records it takes an average of 3.9 seconds. below is the code that I ran to test the conversion speed.

C#

List<Test> d = new List<Test>();
List<AnotherTest> s = new List<AnotherTest>();

for (int i = 0; i < 999999; i++)
{
 d.Add(new Test { ID = i, Name = "Simon", Email = "simon@whale.com" });
}

Stopwatch ss = new Stopwatch();
ss.Start();

d.ToList(s);
ss.Stop();

double r = ss.Elapsed.TotalMilliseconds;

Now that I was happy with this sort of conversion I decided to clean up the code and add some structure. I move the part that created a map into it's own internal class as shown below.

C#

internal static class Mapping
{
    public static IDictionary<PropertyDescriptor, PropertyDescriptor> CreateMap<T, X>(T Source, X Destination)
    {
        PropertyDescriptorCollection SourceProperties = TypeDescriptor.GetProperties(typeof(T));
        PropertyDescriptorCollection DestinationProperties = TypeDescriptor.GetProperties(typeof(X));
        IDictionary<PropertyDescriptor, PropertyDescriptor> Map = new Dictionary<PropertyDescriptor, PropertyDescriptor>();

        //create a map using a dictionary
        foreach (PropertyDescriptor src in SourceProperties)
        {
            PropertyDescriptor dest = DestinationProperties.Find(src.Name, true);
            if (dest != null)
            {
                if (dest.PropertyType == src.PropertyType)
                {
                    //add to dictionary
                    Map.Add(src, dest);
                }
                else
                {
                    //test for nullable types
                    Type SourceNullable = Nullable.GetUnderlyingType(src.PropertyType);
                    Type DestNullable = Nullable.GetUnderlyingType(dest.PropertyType);

                    if (dest.PropertyType.Equals(SourceNullable) == true || src.PropertyType.Equals(DestNullable) == true)
                    {
                        Map.Add(src, dest);
                    }
                }
            }
        }

        return Map;
    }
}

I then changed the ToListConverter to the following

C#

public static void ToList<X, T>(this IEnumerable<T> Source, IList<X> Destintation) where X : new()
        {
            IDictionary<PropertyDescriptor, PropertyDescriptor> Map = Mapping.CreateMap(Source, Destintation);

            foreach (T item in Source)
            {
                object record = Activator.CreateInstance(typeof(X));

                foreach (KeyValuePair<PropertyDescriptor, PropertyDescriptor> mapItem in Map)
                {
                    PropertyDescriptor s = mapItem.Key;
                    PropertyDescriptor d = mapItem.Value;

                    d.SetValue(record, s.GetValue(item));
                }

                Destintation.Add((X)record);
            }
        }

When I test the converter again with the same 1,000,000 rows the speed of the routine increase significantly as it takes an average of 400 ms to complete. BUT I don't know why and would like to.

Can anyone help me understand why this sort of performance would increase by moving the part that creates a dictionary to its own class?

Many thanks and apologies for such a long winded question.

Simon

Posted 6-Nov-13 0:41am

Simon_Whale

Updated 6-Nov-13 0:43am

v2

Add a Solution

Comments

Andreas Gieriet 6-Nov-13 6:49am

How about measuring with a decent tool. The few hundred bugs to buy such a tool safes you many fold your development time where you guess what the bottlenecks are.
Cheers
Andi

Simon_Whale 6-Nov-13 6:53am

there isn't a bottle neck. from the top code section it took 4 seconds to convert 1 million records from class type A to class type B. move the section that creates a map into its own class and it drops that time to 300 - 400 milliseconds and I am trying to understand why this would happen by making that change

Andreas Gieriet 6-Nov-13 7:34am

The "slow" version has obviously some "bottleneck" compared to the "fast" one, agreed? Measure it, don't guess! You need a more sophisticated tool than a stop watch to *analyze* why something goes faster than the other. If you don't like the term bottleneck, you may choose any word you like, e.g. "takes longer", "loops too often", "unnecessarily re-calculates the same value", "has a deeper call stack", ... ;-) You name them.
Cheers
Andi

Simon_Whale 6-Nov-13 7:40am

apologies Andreas I took the word bottleneck in the wrong context as I was more trying to understand why the second version is quicker, rather than just plain accepting it. as my confusion is the number of loops haven't increased, the number of lines haven't really changed apart from the first loop going into a method of its own class etc.

Andreas Gieriet 6-Nov-13 8:20am

Maybe you see the JIT compiler in your stop-watch measurment.
Do a d.ToList(s) once before measuring with the stop whatch.
Cheers
Andi

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Chris Ross 2 · Answer 1 · 2013-11-06T02:09:00

Take a look at the field values of the generated list elements from the second test...

I made a test program from your posted code - got a ten-fold speed improvement. Having a performance profiling tool (check out Red-Gate.com) I could narrow down where in the conversion function the time was / was not being spent: in your old & new versions of the function, add the following lines right before the foreach (var item in Source) loop:

C#

Console.WriteLine("Before: Source.Count: " + Source.Count());
Console.WriteLine("Before: Map.Count: " + Map.Count());

(of course, having 'After' in place of 'Before' where appropriate :)).

I think that'll get you on the way to understanding and fixing your problem.

Totally unrelated..
... and possibly impertinent but possibly helpful: what you're doing, with PropertyDescriptors, is an 'expensive' operation (which you know already). Are you able to require that the source and destination types both implement a common interface (or, simpler, a common abstract base class)?

You wouldn't enforce this constraint in the generic type constraints clause ... but in the static constructor for the class that contains your conversion function (where you would also do, once, any necessary reflection work). Sure, this constraint is enforced at run time, not compile time, but it might

If you can make that constraint you could then also require that the interface requires a Clone() method be implemented - meaning that you'd have no reflection work at all.

Here's a VERY rough idea of the archicture (not compiled, not tested - exists to illustrate the idea):

C#

// ITransform - ensures that derived types are able to copy themselves; probably could use a better name!
interface ITransform
    {
    ITransform Transform(ITransform src);
    }

// For each set of compatible sources/destinations, this declares the common ground
interface  ICustomCopyable : ITransform
    {
    // Required copyable properties for the inheriting real classes
    // ...
    }

// Actual transformable types must satisfy their inherited property constrains AND 
// must implement the ability to be transformable.
class ASource : ICustomCopyable 
    {
    // The fields, properties, methods, etc. that the type really exists for
    //...

    // Look at co-variance / contra-variance; you might be able to implement this with more-derived parameter and result type???
    // Also, look at making this method an extension method - saves duplicating it in each consuming class. BUT, with luck, that's where co/contra variance should be your friend...
    public ITransform Transform(ITransform src)
        {
        // ...
        }
    }

static class ClonableExtension
    {
    public static IList<tdest,> DuplicateList(this IEnumerable<tsrc> src, IList<tdest> dst)
        where TDest : ICustomCopyable, new()
        where TSrc  : ICustomCopyable
        {
        // Iterate src, using srcItem.Clone() to generate its destination counterpart
        }
    }

</tdest></tsrc>

Andreas Gieriet · Answer 2 · 2013-11-06T02:43:00

Solution 2

Unrelated, but still: I fully agree with Chris Ross:
Don't use Reflection without cause.

If you want to have a generic version for any kind of unrelated types, pass a create Func to your ToList,
e.g.

C#

public static IList<TDestination> ToList<TSource, TDestination>(this IEnumerable<TSource> thisSource, Func<TSource, TDestination> delCreate)
{
    return thisSource.Select(e=>delCreate(e)).ToList();
}

But then, why at all to wrap it into a function that doesn not abstract anything useful: call the ...Select(...).ToList() directly in the client code. Your solution seems to re-invent something that can be done in a simple one-liner. Or am I missing something?
Cheers
Andi

Posted 6-Nov-13 2:43am

Andreas Gieriet

Updated 6-Nov-13 3:00am

v4

Comments

Simon_Whale 6-Nov-13 11:23am

Andreas, thank you for your help today.

I found the problem and feel rather stupid, When I took the code into its own class The mapping method didn't actually read the properties of the types passed and built an empty dictionary, this has now been fixed and the results for 1 million rows are back to just over 3.5 seconds which I actually confirmed work.

The reason that I have this routine, is so that I can retrieve information from the database convert it into Business Objects or Data Transfer Objects allowing me to perform some intensive calculations in business logic layer of an application that we use in house. Just as a exercise I was seeing if I could improve either or the performance of the conversion routine or the structure of the code. The tests that I was doing here were around 1000 - 2000% higher in volume than the application actually uses.

But now thinking of this I think that the name of the extension could be renamed to something more self explanatory rather than toList.

need help to understand why the performance increases in my code

2 solutions

Solution 1

Solution 2

Add your solution here

Preview 0

Existing Members

...or Join us