7

This is a question related to C# MVC2 Jqgrid - what is the correct way to do server side paging? where I asked and found how to improve the performance for a query on a table with 2000 or so rows. The performance was improved from 10 seconds to 1 second.

Now I'm trying to perform the exact same query where the table has 20,000 rows - and the query takes 30 seconds. How can I improve it further? 20 thousand rows is still not a huge number at all.

Some possible ideas I have are:

  • It can be improved by de-normalizing to remove joins and sums
  • Make a view and query that instead of querying and joining the table
  • Don't query the whole table, make the user select some filter first (eg an A | B | C .. etc filter)
  • Add more indexes to the tables
  • Something else?

This is the MVC action that takes 30 seconds for 20 thousand rows: (the parameters are supplied by jqgrid, where sidx = which sort column, and sord = the sort order)

public ActionResult GetProductList(int page, int rows, string sidx, string sord, 
string searchOper, string searchField, string searchString)
{
    if (sidx == "Id") { sidx = "ProductCode"; }
    var pagedData = _productService.GetPaged(sidx, sord, page, rows);
    var model = (from p in pagedData.Page<Product>()
            select new
            {
                p.Id, p.ProductCode, p.ProductDescription,
                Barcode = p.Barcode ?? string.Empty, 
                UnitOfMeasure = p.UnitOfMeasure != null ? p.UnitOfMeasure.Name : "",
                p.PackSize, 
                AllocatedQty = p.WarehouseProducts.Sum(wp => wp.AllocatedQuantity),
                QtyOnHand = p.WarehouseProducts.Sum(wp => wp.OnHandQuantity)
            });

    var jsonData = new
    {
        Total = pagedData.TotalPages, Page = pagedData.PageNumber,
        Records = pagedData.RecordCount, Rows = model
    };

    return Json(jsonData, JsonRequestBehavior.AllowGet);
}

ProductService.GetPaged() calls ProductRepository.GetPaged which calls genericRepository.GetPaged() which does this:

public ListPage GetPaged(string sidx, string sord, int page, int rows)
{
    var list = GetQuery().OrderBy(sidx + " " + sord);
    int totalRecords = list.Count();

    var listPage = new ListPage
    {
        TotalPages = (totalRecords + rows - 1) / rows,
        PageNumber = page,
        RecordCount = totalRecords,
    };

    listPage.SetPageData(list
        .Skip((page > 0 ? page - 1 : 0) * rows)
        .Take(rows).AsQueryable());

    return listPage;
}

The .OrderBy() clause uses LinqExtensions so that I can pass in a string instead of a predicate - could this be slowing it down?

And finally ListPage is just a class for conveniently wrapping up the properties that jqgrid needs for paging:

public class ListPage
{
    private IQueryable _data;
    public int TotalPages { get; set; }
    public int PageNumber { get; set; }
    public int RecordCount { get; set; }

    public void SetPageData<T>(IQueryable<T> data) 
    {
        _data = data;
    }

    public IQueryable<T> Page<T>()
    {
        return (IQueryable<T>)_data;
    }
}

GetQuery is:

public IQueryable<T> GetQuery()
{
    return ObjectSet.AsQueryable();
}

The custom .OrderBy method consists of these two methods:

public static IQueryable<T> OrderBy<T>(this IQueryable<T> source, 
    string ordering, params object[] values)
{
    return (IQueryable<T>)OrderBy((IQueryable)source, ordering, values);
}

public static IQueryable OrderBy(this IQueryable source, string ordering, 
    params object[] values)
{
    if (source == null) throw new ArgumentNullException("source");
    if (ordering == null) throw new ArgumentNullException("ordering");
    ParameterExpression[] parameters = new ParameterExpression[] {
        Expression.Parameter(source.ElementType, "") };
    ExpressionParser parser = new ExpressionParser(parameters, ordering, values);
    IEnumerable<DynamicOrdering> orderings = parser.ParseOrdering();
    Expression queryExpr = source.Expression;
    string methodAsc = "OrderBy";
    string methodDesc = "OrderByDescending";
    foreach (DynamicOrdering o in orderings)
    {
        queryExpr = Expression.Call(
            typeof(Queryable), o.Ascending ? methodAsc : methodDesc,
            new Type[] { source.ElementType, o.Selector.Type },
            queryExpr, Expression.Quote(Expression.Lambda(o.Selector, parameters)));
        methodAsc = "ThenBy";
        methodDesc = "ThenByDescending";
    }
    return source.Provider.CreateQuery(queryExpr);
}
Community
  • 1
  • 1
JK.
  • 20,010
  • 29
  • 124
  • 204
  • With the edit, I'm very confused as to how it jumps back to generic-based code, when that OrderBy uses non-generic code... can you clarify? am I missing something obvious? – Marc Gravell Nov 20 '10 at 03:32
  • Sorry I'm not sure what you are asking - this `OrderBy()` is an extension method defined in another class. – JK. Nov 20 '10 at 03:34
  • Well, `GetQuery()` is going to return `IQueryable` (generic); `GetQuery().OrderBy(sidx + " " + sord)` (using the method you show) will be `IQueryable` (non-generic). AFAIK there is no `Skip(...)` etc *defined* for non-generic... not sure how that is going to be working? – Marc Gravell Nov 20 '10 at 03:37
  • Also - silly question, but is there an *index* defined for the sort you are trying to do? Could this be as simple as adding an index to the database column? – Marc Gravell Nov 20 '10 at 03:38
  • Good question about the generic vs non generic. I dont know, but when I hover the mouse over the .OrderBy it says it is `(extension) IQuerable` and it also says that `var list ` is `IQueryable`. There is no index on the ProductCode column - it is nvarchar(500) so too large for an index as I understand it. – JK. Nov 20 '10 at 03:45
  • @JK - the index will surely help, but I don't think that the one you have posted is the right `OrderBy` method; you might want to try [F12] (Go To Definition) – Marc Gravell Nov 20 '10 at 03:49
  • Ah right F12 shows how it goes from generic to non to back again. I will edit the question again (how many edits am I allowed?) – JK. Nov 20 '10 at 04:07

1 Answers1

6

The bit that concerns me is:

.Take(rows).AsQueryable()

the fact that you needed to add AsQueryable() suggests to me that it is currently IEnumerable<T>, meaning you could be doing the paging at the wrong end of the query (bringing back way too much data over the network). Without GetQuery() and the custom OrderBy() it is hard to be sure - but as always, the first thing to do is to profile the query via a trace. See what query is executed and what data is returned. EFProf might make this easy, but a SQL trace is probably sufficient.

Marc Gravell
  • 927,783
  • 236
  • 2,422
  • 2,784
  • 1
    +1 - also there is a little known `ObjectQuery` ext meth called `.ToTraceString()` which shows the SQL query to be executed. Very handy for logging. – RPM1984 Nov 20 '10 at 03:24
  • Sorry I missed that, `GetQuery()` is a one liner: `return ObjectSet.AsQueryable();` . I will edit and add the code for OrderBy() - I don't know where it was sourced from, possibly here: linqextensions.codeplex.com/ – JK. Nov 20 '10 at 03:25
  • @JK - to be honest, it might suffice for you to simply check if the code works if you remove the `.AsQueryable()` – Marc Gravell Nov 20 '10 at 03:28
  • @JK - perhaps [here](http://stackoverflow.com/questions/41244/dynamic-linq-orderby/233505#233505) ? – Marc Gravell Nov 20 '10 at 03:29
  • It still compiles without `AsQueryable`. I will do a timing test, it will take a few mins to get it all setup and done. – JK. Nov 20 '10 at 03:30
  • @JK - it is more the actual query that goes to the DB that will be interesting here. – Marc Gravell Nov 20 '10 at 03:35
  • Where can I add the `.ToTraceString()`? I don't have any variables of type `ObjectQuery` – JK. Nov 20 '10 at 03:51
  • @RPM1984 can you advise on the above comment at all? – Marc Gravell Nov 20 '10 at 03:53
  • It still works very quickly on the 2k list without the `AsQueryable()`. But there's something wrong with my test database with 20k products. I will have to reimport the CSV before I can see how long it takes. – JK. Nov 20 '10 at 03:59
  • 1
    @Marc Gravell - `ObjectQuery` implements `IQueryable`. `ObjectQuery` is EF-specific, and since he's working with EF, he can do an explicit cast to `ObjectQuery`: `var trace = ((ObjectQuery)query).ToTraceString()`. He's doing `ObjectSet.AsQueryable()`. If he *didn't* do the `AsQueryable`, and did some LINQ operations on `ObjectSet`, the result would be an `ObjectQuery`. Am i wrong? – RPM1984 Nov 20 '10 at 04:15
  • Where is the best place to put `ToTraceString`? I added it to `GetPaged` directly after the `GetQuery.OrderBy()`. The trace just said `SELECT Id, ProductCode, .... FROM Products ORDER BY ProductCode`. I do notice that it selected all fields even though I only use a few of them. – JK. Nov 20 '10 at 04:23
  • So it worked. Well i guess i *wasnt* wrong. :) The best place to put it is right before you execute the query (`.ToList(), `SingleOrDefault()`, etc). I would do it in your `GetPaged` method: `var trace = ((ObjectQuery)GetQuery().OrderBy(sidx + " " + sord)).ToTraceString();` – RPM1984 Nov 20 '10 at 04:43
  • CSV import has only processed 6500 rows (its very slow!), but all indications are good so far - taking just 1 second to load the page. The only change I've made is to remove the `AsQueryable()`. I think that was only there because originally ListPage.Page was a different type (I forget what now). – JK. Nov 20 '10 at 04:47
  • Well, that was a pretty good answer - accepted with thanks. 22k records are now paging in just 1 second. It was pretty sharp of you to pick out the `AsQuerable()` problem amongst all that code - straight to the correct answer. – JK. Nov 20 '10 at 05:20