3
\$\begingroup\$

The following code takes about 20 seconds to run with about 200,000 records in the TaskLogs table:

using (var db = new DAL.JobManagerEntities())
    {
        return db.TaskLogs.Select(taskLog => new TaskLog()
        {
            TaskLogID = taskLog.TaskLogID,
            TaskID = taskLog.TaskID,
            TaskDescription = taskLog.Task.TaskDescription,
            TaskType = (TaskType)taskLog.Task.TaskTypeID,
            RunID = taskLog.RunID,
            ProcessID = taskLog.ProcessID,
            JobID = taskLog.JobID,
            JobName = taskLog.Job.JobName,
            Result = taskLog.Result,
            StartTime = taskLog.StartTime,
            TimeTaken = taskLog.TimeTaken
        }).OrderByDescending(t => t.RunID).ThenByDescending(t => t.RunID).ThenByDescending(t => t.StartTime).ToList();
    }

I tweaked it until I got something that runs faster. Here's where I got to:

            using (var db = new DAL.JobManagerEntities())
    {
        db.Configuration.LazyLoadingEnabled = false;
        var tasks = db.Tasks.ToList();
        var jobs = db.Jobs.ToList();
        var result = db.TaskLogs.Select(x => new TaskLog()
        {
            TaskLogID = x.TaskLogID,
            TaskID = x.TaskID,
            RunID = x.RunID,
            ProcessID = x.ProcessID,
            JobID = x.JobID,
            Result = x.Result,
            StartTime = x.StartTime,
            TimeTaken = x.TimeTaken
        }).OrderByDescending(t => t.RunID).ThenByDescending(t => t.StartTime).ToList();

        foreach (var r in result)
        {
            r.TaskDescription = tasks.Single(t => t.TaskID == r.TaskID).TaskDescription;
            r.TaskType = (TaskType)tasks.Single(t => t.TaskID == r.TaskID).TaskTypeID;
            r.JobName = jobs.Single(j => j.JobID == r.JobID).JobName;
        }
        return result;}

Which runs in less than 6 seconds for the same number of records.

The TaskLog table is linked to the Job and Task tables as follows: enter image description here

The Job and Task tables will have 100s and 1000s of records respectively.

Is there anything else I could do in order to further improve the efficiency of the code?

\$\endgroup\$
9
  • \$\begingroup\$ I don't see why you think you need LazyLoading in the first solution. Things like server side taskLog.Task.TaskDescription will be handled just fine without lazy loading. Can you disable LazyLoading and Proxy creation for the first sample as well as using AsNoTracking? Would be interesting whether this boosts performance. \$\endgroup\$
    – grek40
    Feb 14, 2018 at 11:42
  • \$\begingroup\$ That improved the performance by about 1.5 seconds, but still nothing compared to the second version of the code, especially when combined with the answer from @JanDotNet \$\endgroup\$
    – Kappacake
    Feb 15, 2018 at 10:17
  • \$\begingroup\$ This is very strange... is there any noteworthy difference when you create an anonymous result type (Select(taskLog => new { ... }) instead of new TaskLog() ...) and don't cast the TaskTypeID but take it as the returned type? Below you commented that the query runs in like 15ms, rest being the ToList... how did you test this, it sounds a bit fast? \$\endgroup\$
    – grek40
    Feb 15, 2018 at 10:27
  • \$\begingroup\$ Did you check the generated query (inspect it eg. with db.Database.Log = x => WriteSomewhere(x) and check query performance with some dedicated SQL tool for comparison) \$\endgroup\$
    – grek40
    Feb 15, 2018 at 10:34
  • 1
    \$\begingroup\$ Ok, so worst case, JobName and TaskDescription sum up to 1000 characters per record, which is more than all the other properties combined. Since you mention there will be a lot less entries (1000s vs 200000 records), the solution from @JanDotNet makes perfect sense, it will just transfer a lot less duplicated strings from server to client. \$\endgroup\$
    – grek40
    Feb 15, 2018 at 11:38

2 Answers 2

3
\$\begingroup\$

1) Not sure if it impacts performance, but your first code fragment has one redunant order by:

.OrderByDescending(t => t.RunID).ThenByDescending(t => t.RunID)

2) You could improve performance of your seconds code with "client side indexing" (using a dictionary):

var tasksMap = tasks.ToDictionary(t => t.TaskID);
var jobsMap = jobs.ToDictionary(t => t.JobID);

foreach (var r in result)
{
    var task = tasksMap[r.TaskID];
    r.TaskDescription = task.TaskDescription;
    r.TaskType = (TaskType)task.TaskTypeID;
    r.JobName = jobsMap[r.JobID].JobName;
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the answer. Would var taskMap = db.Tasks.ToDictionary(t => t.TaskID) be the same as var tasks = db.Tasks.ToList(); var taskMap = tasks.ToDictionary(t => t.TaskID)? \$\endgroup\$
    – Kappacake
    Feb 12, 2018 at 20:28
  • \$\begingroup\$ Not sure, but I'd expect it. \$\endgroup\$
    – JanDotNet
    Feb 12, 2018 at 20:45
  • \$\begingroup\$ The client side indexing does improve performance indeed. Execution time for the foreach loop went from about 500ms to about 30ms! \$\endgroup\$
    – Kappacake
    Feb 14, 2018 at 10:47
2
\$\begingroup\$

You can do two things to speed this query up:

  • use joins so that the entire query runs on the server and you don't have to run over the results again with the foreach loop
  • add .AsNoTracking() to each table so that EF does not have to create proxy objects for change-tracking.

Here's an example of how it would look like when we apply both suggestions.

using (var db = new DAL.JobManagerEntities())
{
    db.Configuration.LazyLoadingEnabled = false;

    var result = 
        from taskLog in db.TaskLogs.AsNoTracking()
        join task in db.Tasks.AsNoTracking() on taskLog.TaskID equals task.TaskID
        join job in db.Jobs.AsNoTracking() on taskLog.JobID equals job.JobID
        orderby taskLog.RunID descending, taskLog.StartTime descending
        select new TaskLog
        {
            TaskLogID = taskLog.TaskLogID,
            TaskID = taskLog.TaskID,
            RunID = taskLog.RunID,
            ProcessID = taskLog.ProcessID,
            JobID = taskLog.JobID,
            Result = taskLog.Result,
            StartTime = taskLog.StartTime,
            TimeTaken = taskLog.TimeTaken,
            TaskDescription = task.TaskDescription,
            TaskType = (TaskType)task.TaskTypeID,
            JobName = job.JobName
        };


    return result.ToList();
}
\$\endgroup\$
13
  • \$\begingroup\$ Thanks a lot for the answet! I’m going to try this later on and let you know \$\endgroup\$
    – Kappacake
    Feb 14, 2018 at 7:56
  • \$\begingroup\$ I had to make a few changes to make the above code work: join job in db.Jobs.AsNoTracking() on taskLog.JobID equals job.TaskID needs to be join job in db.Jobs.AsNoTracking() on taskLog.JobID equals job.JobID and I also need to order the result before returning return result.OrderByDescending(t => t.RunID).ThenByDescending(t => t.StartTime).ToList();. Unfortunately this code runs in about 20 seconds, so it is much slower than the original one. \$\endgroup\$
    – Kappacake
    Feb 14, 2018 at 10:03
  • \$\begingroup\$ @demonicdaron Yeah, I thougt so that I might have the IDs wrong - but this was notepad-coding ;-] If this is even slower then most probably because you order it again. There is already an orderby that will run on the server. No need to do it locally again. \$\endgroup\$
    – t3chb0t
    Feb 14, 2018 at 10:07
  • \$\begingroup\$ sorry I completely missed that order by! for some reasons it wasn't returning the same data as the original code, so I added the order by and that corrected the problem. I'm gonna do some more testing and come back to your answer. Thanks for the help \$\endgroup\$
    – Kappacake
    Feb 14, 2018 at 10:11
  • \$\begingroup\$ I just tried removing the order by altogether (both from the query and the return), but it made little difference. I also removed the where clause, but that also had little effect. \$\endgroup\$
    – Kappacake
    Feb 14, 2018 at 10:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.