Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Refactoring - Speed increase

How can I make this function more efficient. It's currently running at 6 - 45 seconds. I've ran dotTrace profiler on this specific method, and it's total time is anywhere between 6,000ms to 45,000ms. The majority of the time is spent on the "MoveNext" and "GetEnumerator" calls.

and example of the times are

71.55% CreateTableFromReportDataColumns - 18, 533* ms - 190 calls
 -- 55.71% MoveNext - 14,422ms - 10,775 calls 

can I do to speed this method up? it gets called a lot, and the seconds add up:

    private static DataTable CreateTableFromReportDataColumns(Report report)
    {
        DataTable table = new DataTable();
        HashSet<String> colsToAdd = new HashSet<String> { "DataStream" };
        foreach (ReportData reportData in report.ReportDatas)
        {
            IEnumerable<string> cols = reportData.ReportDataColumns.Where(c => !String.IsNullOrEmpty(c.Name)).Select(x => x.Name).Distinct();

            foreach (var s in cols)
            {
                if (!String.IsNullOrEmpty(s))
                    colsToAdd.Add(s);
            }
        }

        foreach (string col in colsToAdd)
        {
            table.Columns.Add(col);
        }

        return table;
    }

If you need the sql table definitions here they are:

ReportData

ReportID            int

ReportDataColumn

ReportDataColumnId  int
ReportDataId        int 
Name                varchar(255)    
Value               text    
like image 570
Michael G Avatar asked Feb 26 '23 22:02

Michael G


2 Answers

I believe you should be able to simplify your function into something like this

var columnsToAdd = report.ReportDatas
                    .SelectMany(r => r.ReportDataColumns)
                    .Select(rdc => rdc.Name)
                    .Distinct()
                    .Where(name => !string.IsNullOrEmpty(name));

And from there add the names to your table.

like image 101
Anthony Pegram Avatar answered Mar 08 '23 05:03

Anthony Pegram


Your code (only) runs foreach loops so the conclusion that the method spends most of its time in MoveNext() et al is not so surprising.

You are doing double work on both the isnullOrEmpty and the Distinct (is repeated by the HashSet).

My version would be:

private static DataTable CreateTableFromReportDataColumns(Report report)
{
    DataTable table = new DataTable();
    HashSet<String> colsToAdd = new HashSet<String> { "DataStream" };
    foreach (ReportData reportData in report.ReportDatas)
    {

        foreach (var column in reportData.ReportDataColumns)
        {
            if (!String.IsNullOrEmpty(column.Name))
                colsToAdd.Add(column.Name);
        }
    }

    foreach (string col in colsToAdd)
    {
        table.Columns.Add(col);
    }

    return table;
}

But I don't expect a huge improvement

like image 25
Henk Holterman Avatar answered Mar 08 '23 05:03

Henk Holterman