match match = myRepo.GetAll()
.Where(m => m.personId == personId)
.Where(m => m.companyId == companyId).FirstOrDefault();
now there are over 1m records and it takes ages. I refactored it to do a single linq query and it runs instantly. This suggests the the myRepo.GetAll() returns everything then its looping over every row to do the where's rather than do it at the db level.
GetAll looks like this:
public virtual IEnumerable<T> GetAll(Paging p = null)
{
// Get the set as a queryable.
IQueryable<T> q = _db.Set<T>();
if(p != null)
{
p.TotalCount = q.Count();
q = q.Skip(p.StartAt).Take(p.PageSize);
}
// Return the enumerable.
return q.AsEnumerable<T>();
}
I just want to sanity check if my theory is correct, some of the guys think because its IEnumerable it shouldnt do this, but its returning that first then has to loop over in the local function in my opinion as the where's are outside the linq query.
The fact that your GetAll
method returns an IEnumerable<T>
(rather than IQueryable<T>
) means that your Where
call is using LINQ to Objects, not EF. In other words, that query is fetching all the data locally, and filtering it in your process. That's not good.
Consider changing GetAll
to:
public virtual IQueryable<T> GetAll(Paging p = null)
{
// Get the set as a queryable.
IQueryable<T> q = _db.Set<T>();
if(p != null)
{
p.TotalCount = q.Count();
q = q.Skip(p.StartAt).Take(p.PageSize);
}
return q;
}
Then you're still in "queryable land". Note that this is still flawed though, as you'd want to apply paging after any other queries, not before. So you might actually want to just have a method returning _db.Set<T>()
, and then have a separate extension method for paging. (I'm nervous of the fact that your paging implementation requires you to count the full results, mind you...)
See more on this question at Stackoverflow