Issue with retrieving items from an IEnumerable<T>

RichardGrimmer

I have a class with 11 string properties, together with an IEnumerable (Initialised to a generic List) property, each item of which has an int property, a long property and 3x Nullable int (int?) properties. The typical number of items in this sub property list is 4-6. I have an IEnumerable containing ~700K of these, and I'm breaking them out into batches of 1000 using the following code;

var enumerator = items.GetEnumerator();

enumerator.MoveNext();

List<object> entityBatch = new List<object>();

DateTime start = DateTime.Now.Ticks;

for (int iterStep = 0; iterStep < batchSize; iterStep++)
{
entityBatch.Add(enumerator.Current);
enumerator.MoveNext();
}

DateTime end = DateTime.Now.Ticks;

However, to process this loop is taking around 12 seconds (as measured using the start / end tick count)....that in itself is bad, but the real question is can anybody shed any light on the fact that if I remove the list property, the same data is processed in 0.25 seconds - so several orders of magnitude more quickly? I realise there will be some casting going on from objects to my actual types, but the difference makes me think I'm missing something pretty significant.

C# has already designed away most of the tedium of C++.

Richard Deeming

Don't measure the performance of your code using DateTime; use the Stopwatch class[^] instead. Did you make sure to "warm up" the code before you timed it, and run the test over many iterations? Since you know the maximum number of items you'll be adding to the list, you should set its capacity when you create it:

List<object> entityBatch = new List<object>(batchSize);

If you don't set the capacity, it starts at 4 and doubles each time it runs out of space. You need to check the value returned from enumerator.MoveNext; if it returns false, you've reached the end of the sequence. Your code currently continues adding the final items from the sequence to the list until you reach the batch size. You should consider using proper generic types, rather than a List<object>; that way, you avoid having to cast the items back to the correct type when you read the batches. It's probably best to wrap this sort of thing up as a extension method. Here's the one I use:

public static class EnumerableExtensions
{
private static IEnumerable<IEnumerable<TSource>> BatchIterator<TSource>(IEnumerable<TSource> source, int size)
{
int count = 0;
var bucket = new TSource[size];
foreach (var item in source)
{
bucket[count] = item;

        checked
        {
            count++;
        }

        if (count == size)
        {
            yield return bucket.Select(x => x);
            bucket = new TSource\[size\];
            count = 0;
        }
    }
    if (count != 0)
    {
        yield return bucket.Take(count);
    }

    // ReSharper disable once RedundantAssignment
    bucket = null;
}

public static IEnumerable<IEnumerable<TSource>> Batch<TSource>(this IEnumerable<TSource> source, int size)
{
    if (source == null) throw new ArgumentNullException(nameof(source));
    if (size < 1) throw new ArgumentOutOfRangeException(nameof(size));
    return BatchIterator(source, size);
}

}

If you can, try to reproduce the problem in a small e

RichardGrimmer

Hi Richard - many thanks for the quick reply...

Richard Deeming wrote:

Don't measure the performance of your code using DateTime; use the Stopwatch class[^] instead.

I'm not measuring the performance per-se, just getting an idea, and in any case that part of the code won't be in the prod version...

Richard Deeming wrote:

Did you make sure to "warm up" the code before you timed it, and run the test over many iterations?

Absolutely - same perf over many MANY runs :)

Richard Deeming wrote:

Since you know the maximum number of items you'll be adding to the list, you should set its capacity when you create it:

List<object> entityBatch = new List<object>(batchSize);

If you don't set the capacity, it starts at 4 and doubles each time it runs out of space.

Valid point - made the change and no real effect - still around 12s

Richard Deeming wrote:

You need to check the value returned from enumerator.MoveNext; if it returns false, you've reached the end of the sequence. Your code currently continues adding the final items from the sequence to the list until you reach the batch size.

Again, valid point, but in this instance it's not the problem - I can be certain that it's not at the end.

Richard Deeming wrote:

You should consider using proper generic types, rather than a List<object>; that way, you avoid having to cast the items back to the correct type when you read the batches.

Also a valid point, but not possible in this instance - it's in an abstract base class, which operates over several types of similar but not related types and exists in a (necessarily) singleton class....my thinking is leading me to the point that it may well be an area to look at, but given that IEnumerator.Current returns an object type anyway I'm not sure.

Richard Deeming wrote:

It's probably best to wrap this sort of t

Richard Deeming

RichardGrimmer wrote:

not possible in this instance - it's in an abstract base class, which operates over several types of similar but not related types and exists in a (necessarily) singleton class....

So you can't do something like this?

public abstract class MyBaseClass
{
public void ProcessList<T>(IEnumerable<T> listToProcess) where T : MyBaseClass
{
...
}
}

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

RichardGrimmer

Unfortunately not (admittedly not thought it all the way through, but the types in question don't share a common base class)....I do appreciate that I could re-structure so that they do however, but again, given the fact that the problem manifests itself a s difference of several orders of magnitude, I'm in a "bigger fish to fry" type of situation - thanks again for the suggestion however - I may well end up doing something similar "just for grins" and see what happens....just for background, the code is used to index items from several different data sources into an Elastic Search instance, hence the object type being used....

C# has already designed away most of the tedium of C++.

Pete OHanlon

What do your classes actually look like? I took your description and built these out:

public class ChildItem
{
public ChildItem()
{
Count = int.MaxValue;
LongCount = long.MaxValue;
Nullable2 = int.MaxValue - 3;
Nullable3 = Nullable2.Value - 2;
}
public int Count { get; set; }
public long LongCount { get; set; }
public int? Nullable1 { get; set; }
public int? Nullable2 { get; set; }
public int? Nullable3 { get; set; }
}
public class MasterList
{
private List<ChildItem> children = new List<ChildItem>();
public MasterList()
{
for (int i = 0; i < 6; i++)
{
children.Add(new ChildItem());
}
Master1 = "Hi";
Master2 = "Hi";
Master3 = "Hi";
Master4 = "Hi";
Master5 = "Hi";
Master6 = "Hi";
Master7 = "Hi";
Master8 = "Hi";
Master9 = "Hi";
Master10 = "Hi";
Master11 = "Hi";
Master12 = "Hi";
}

public IEnumerable<ChildItem> Children => children;

public string Master1 { get; set; }
public string Master2 { get; set; }
public string Master3 { get; set; }
public string Master4 { get; set; }
public string Master5 { get; set; }
public string Master6 { get; set; }
public string Master7 { get; set; }
public string Master8 { get; set; }
public string Master9 { get; set; }
public string Master10 { get; set; }
public string Master11 { get; set; }
public string Master12 { get; set; }
}
public class TestIt
{
private List<MasterList> items = new List<MasterList>(700000);

public TestIt()
{
for (int i = 0; i < 700000; i++)
{
items.Add(new MasterList());
}
Console.WriteLine("Ready");
}

public void StartTest(string text = "Dummy run")
{
var enumerator = items.GetEnumerator();

enumerator.MoveNext();

List<object> entityBatch = new List<object>();

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

for (int iterStep = 0; iterStep < 700000; iterStep++)
{
  entityBatch.Add(enumerator.Current);
  enumerator.MoveNext();
}
sw.Stop();

Console.WriteLine(text);
Console.WriteLine("Total time {0}", sw.ElapsedMilliseconds);

}
}

Running this through a few times consistently returns a time of less than 50 milliseconds.

This space for rent

RichardGrimmer

Hi Pete, So the classes in question represent a name and a collection of dates of birth associated with the name...classes below;

public partial class DateOfBirth
{
public int DateOfBirthId { get; set; }
public Nullable Day { get; set; }
public Nullable Month { get; set; }
public Nullable Year { get; set; }
public long PepDeskRecordId { get; set; }
}

public class ExpandedFoo
{

    /// /// Gets or sets a value indicating the Foo Id associated with this person
    /// 
    public string FooId { get; set; }

    /// /// Gets or sets a value indicating the Bar Id (Primary Key) associated with this person
    /// 
    public string BarId { get; set; }

    /// /// Gets or sets a value indicating the title of this person
    /// 
    public string Title { get; set; }

    /// /// Gets or sets a value indicating the gender of this person
    /// 
    public string Gender { get; set; }

    /// /// Gets or sets a value indicating the first name of this person
    /// 
    public string FirstName { get; set; }

    /// /// Gets or sets a value indicating the last name of this person
    /// 
    public string LastName { get; set; }

    /// /// Gets or sets a value indicating the full name of this person
    /// 
    public string FullName { get; set; }

    /// /// Gets or sets a value indicating the other names associated with this person
    /// 
    public string OtherNames { get; set; }

    /// /// Gets or sets a value indicating the Country associated with this person
    /// 
    public string Country { get; set; }

    /// /// Gets or sets a value indicating the Blah Id associated with this person
    /// 
    public string BlahId{ get; set; }

    /// /// Gets or sets a value indicating the Combined name associated with this person
    /// 
    public string CombinedName { get; set; }

    /// /// Gets or sets a value indicating the date of births associated with the Foo
    /// 
    p

RichardGrimmer

OK - I've cracked the issue, but I'm not even going to pretend I know why...it appears it was down to how I was populating my lists.... The pop code originally looked like;

var expandedEntities = Foontities.Select(entity => new ExpandedFoo
{
FooId = entity.fooId.ToString(CultureInfo.InvariantCulture),
BarId = entity.BarId.ToString(CultureInfo.InvariantCulture),

            Title = entity.Title,
            Gender = entity.Gender,
            FirstName = entity.FirstName,
            LastName = entity.LastName,
            FullName = entity.FullName,

            OtherNames = entity.OtherNames,
            CombinedName = entity.CombinedName,

            Country = entity.Country,

            BlahId=  entity.BlahId.ToString(),

            DateOfBirths = (from DateOfBirth dob in dateOfBirths
                            where dob.FooId== entity.FooId
                            select new FooDateOfBirth()
                            {
                                DateOfBirthId = dob.DateOfBirthId,
                                Year = dob.Year,
                                Month = dob.Month,
                                Day = dob.Day
                            }).ToList()
        });

Which was causing the 12s time to extract 1000 records, but (as mentioned, this is to index into Elastic Search), converting to Json and building a bulk command using PlainElasic.net was <.2s.... Removing the ToList(), to make the DOB population code;

DateOfBirths = (from DateOfBirth dob in dateOfBirths
where dob.PooId == entity.FooId
select new FooDateOfBirth()
{
DateOfBirthId = dob.DateOfBirthId,
Year = dob.Year,
Month = dob.Month,
Day = dob.Day
})

Fixed the extraction problem and took the time to milliseconds - BUT caused the Json conversion and bulk command building to take (coincidentally) 12s... HOWEVER, doing;

DateOfBirths = (from DateOfBirth dob in dateOfBirths
where dob.FooId == entity.FooId
select new FooDateOfBirth()
{

RichardGrimmer

Hi Richard, I've just posted an update above - very odd, but thought you might like to take a look - it was (apparently) down to how I was populating my data... Many thanks for all your help and suggestions!

C# has already designed away most of the tedium of C++.

Pete OHanlon

Just as a test, go back to your ExoandedFoo and change the IEnumerable into a List and populate it using the ToList() method.

This space for rent

RichardGrimmer

Hi Pete, Actually I was all sorts of wrong - on deeper investigation, the as statement was failing silently (as it of course should) - a cast rather than the as did indeed throw exceptions left right and centre. Not a problem, I've taken a different tack now...I may at a later date circle back around and look at it in more detail, but for now it'll do as it is. Many MANY thanks for your time on this one

C# has already designed away most of the tedium of C++.