CODEWITHSUNDEEP
c#performancelinqquery-optimization
dinikai
dinikai

Reputation: 13

Why is this method SO slow?

I have a method to paginate records.

public static IEnumerable<T> Paginate<T>(IEnumerable<T> records, int count, int page,
    out int? nextPage, out int? previousPage, out int from, out int to, bool canScroll = true)
{
    nextPage = null;
    previousPage = null;
    if (!canScroll)
    {
        from = 1;
        to = count;
        return records.Take(count);
    }

    if (page > 0)
    {
        previousPage = page - 1;
    }
    if (records.Count() > (page + 1) * count)
    {
        nextPage = page + 1;
    }

    var listed = records.Skip(count * page).Take(count);
    from = count * page + 1;
    to = count * page + listed.Count();
    return listed;
}

records parameter is EF Where() result.

This method takes 10+ (!) seconds while processing table with 500 rows. How can I impove it?

Upvotes: 0

Views: 157

Answers (3)

SinanTaskinlar
SinanTaskinlar

Reputation: 1

Your method is slow because of records.Count(), which executes a separate database query each time. Since records is an EF Where() result, this makes it very inefficient.

store the count in a variable to avoid multiple DB queries:

int totalRecords = records.Count();

use .ToList() only when needed to fetch data into memory:

var listed = records.Skip(count * page).Take(count).ToList();

use OrderBy() before Skip() and Take() for EF to optimize the query:

var listed = records.OrderBy(x => x.Id).Skip(count * page).Take(count).ToList();

Upvotes: -1

mjwills
mjwills

Reputation: 23975

An approach you may want to consider:

public static IEnumerable<T> Paginate<T>(IQueryable<T> records, int count, int page,
    out int? nextPage, out int? previousPage, out int from, out int to, bool canScroll = true)
{
    nextPage = null;
    previousPage = null;
    if (!canScroll)
    {
        // I will ignore the weirdness here for now
        from = 1;
        to = count;
        return records.Take(count);
    }

    if (page > 0)
    {
        previousPage = page - 1;
    }


    var data = records.Skip(count * page).Take(count + 1).ToList();
    var hasNextPage = data.Count > count;

    if (hasNextPage)
    {
        data.RemoveAt(data.Count - 1);
        nextPage = page + 1;
    }

    from = count * page + 1;
    to = count * page + data.Count;
    return data;
}

By requesting one more than the current page (.Take(count + 1)), you more easily determine there is a next page. This code will execute a maximum of one query against the database (your existing code will run more than one query, and those queries will bring much more data over the wire than necessary).

One thing to think on a little is what should happen if they ask for a page of data when there isn't any. Your current code acts a little weirdly in that scenario (e.g. it misleadingly sets from). My code doesn't change that.

Upvotes: 2

Amos
Amos

Reputation: 68

You need to change IEnumerable<T> records to IQueryable<T> records. IEnumerable causes you to load all your database data to memory while IQueryable is SQL. You can verify this by printing the generated SQL log.

In addition, this method does a lot of this loading all the data from the database into memory

  1. records.Count() > (page + 1) * count
  2. var listed = records.Skip(count * page).Take(count), triggered when the method external ToList
  3. to = count * page + listed.Count();

It can be optimized to query data only once

public static IQueryable<T> Paginate<T>(IQueryable<T> records, int count, int page, out int? previousPage, out int from ,bool canScroll = true)
{
    previousPage = null;
    if (!canScroll)
    {
        from = 1;
        return records.Take(count + 1);
    }

    if (page > 0)
    {
        previousPage = page - 1;
    }

    var listed = records.Skip(count * page).Take(count + 1);

    from = count * page + 1;

    return listed;
}

int count = 100;
int page = 0;
int? nextPage = null;
var pageResult = Paginate(records, count, page, out int? previousPage, out int from,  true).ToList();
if (pageResult.Count > count)
{
    nextPage = page + 1;
    pageResult.RemoveAt(pageResult.Count - 1);
}
int to = count * page + pageResult.Count;

Upvotes: 3

Related Questions