Speed up query to rank team sales by aggregating data from two databases

Question

On my machine it runs in about 60 seconds and about 2-3 minutes live.

The criteria needed are:

• Current month only (from first of month until current day)
• I need the rank of the currently logged in person based on total sales.

• I have two tables across two databases where this data is to be gathered:

  • Database 1: ProPit_User table (this houses the logged in users).
  • Database 2: This houses the sales in the Orders table.

• Both tables will be linked on the SalesRepID column.

• There are around around 352 users in the ProPit_User table.
• There are a lot of orders in the orders table.

This code does what I need it to do; it is just dreadfully slow.

public static int GetNationalRank(int teamId, int CurrentUserId)
{
    List<ProPit_User> lstProPit_User = new List<ProPit_User>();
    List<TeamSales> lstTeamSales = new List<TeamSales>();
    int currentUserRank = 0;

    using (ProsPitEntities ppdb = new ProsPitEntities())
    using (TraegerEntities db = new TraegerEntities())
    {
        lstProPit_User = ppdb.ProPit_User.Where(x => x.SalesRepID != null).ToList();

        foreach (var user in lstProPit_User)
        {

            var strStartDate = DateTime.Now.AddDays(-(DateTime.Now.Day - 1)).ToString("M/d/yyyy");
            var dtStartDate = Convert.ToDateTime(strStartDate);

            var strEndDate = DateTime.Now.AddDays(1).ToString("M/d/yyyy");
            var dtEndDate = Convert.ToDateTime(strEndDate);

            var orders = db.Orders.Where(o => o.DateCompleted >= dtStartDate
                                        && o.DateCompleted < dtEndDate
                                        && (o.Status == 1 || o.Status == 2)
                                        && o.Kiosk != 0).AsEnumerable();

            var total = (from o in orders
                         where o.SalesRepID == user.SalesRepID
                         select o.OrderTotal).Sum();

            TeamSales TeamSales = new TeamSales();

            TeamSales.userId = user.userID;
            TeamSales.TotalSales = total;


            lstTeamSales.Add(TeamSales);

            }

        }

        int rank = lstTeamSales.OrderByDescending(x => x.TotalSales).FindIndex(x => x.userId == CurrentUserId);

        if (rank == -1)
        {
            currentUserRank = 0;
        }
        else
        {
            // Zero based index need to add one to rank
            currentUserRank = rank + 1;
        }

    }

    return currentUserRank;
}

I have .FindIndex added as an extension method to IEnumerable.

All I really need is to get the rank of the person logged in in comparison to everyone else sales.

Answer 1

Try what's below on for size. The date calculation is moved out of the loop, the TeamSales list capacity is pre-allocated and finally, the two LINQ statements are combined into one and rolled into AddRange rather than the explicit loop.

public static int GetNationalRank(int teamId, int CurrentUserId)
{
    int currentUserRank;
    var startDate = Convert.ToDateTime(DateTime.Now.AddDays(-(DateTime.Now.Day - 1)).ToString("M/d/yyyy"));
    var endDate = Convert.ToDateTime(DateTime.Now.AddDays(1).ToString("M/d/yyyy"));
    List<ProPit_User> proPitUsers;

    using (var ppdb = new ProsPitEntities())
    {
        proPitUsers = ppdb.ProPit_User.Where(x => x.SalesRepID != null).ToList();
    }

    var teamSales = new List<TeamSales>(proPitUsers.Count());

    using (var db = new TraegerEntities())
    {
        teamSales.AddRange(proPitUsers
            .Select(user => new
            {
                user,
                total = db.Orders.Where(o => o.DateCompleted >= startDate
                    && o.DateCompleted < endDate
                    && (o.Status == 1 || o.Status == 2)
                    && o.Kiosk != 0
                    && o.SalesRepID == user.SalesRepID).Select(o => (decimal?)o.OrderTotal).Sum() ?? 00.0M
            })
            .Select(userTotal => new TeamSales { userId = userTotal.user.userID, TotalSales = userTotal.total }));

        var rank = teamSales
            .OrderByDescending(x => x.TotalSales)
            .FindIndex(x => x.userId == CurrentUserId);

        currentUserRank = rank == -1 ? 0 : rank + 1;
    }

    return currentUserRank;
}

Answer 2

I think your main problem is that for each user in the list you are performing two LINQ queries, at least one of which hits the database.

What you are doing is:

• for each user
  • get all orders (for everyone) and load them into memory
  • select out only the orders you are looking for
  • do stuff with data
  • delete all orders you retrieved so you can just go and get them all again next time

I think you would have more luck if you pulled all of this out of the loop:

        var strEndDate = DateTime.Now.AddDays(1).ToString("M/d/yyyy");
        var dtEndDate = Convert.ToDateTime(strEndDate);

        var orders = db.Orders.Where(o => o.DateCompleted >= dtStartDate
                                    && o.DateCompleted < dtEndDate
                                    && (o.Status == 1 || o.Status == 2)
                                    && o.Kiosk != 0).AsEnumerable();

This way you:

• Query all of the orders for everyone (only once)
• For each user
  • select out only the orders you are looking for
  • do stuff with data

This way you are only getting all of the data only once. Remember that SQL queries are the slowest part of almost any method. You don't want to query more information than you need to (select Col1, Col2 is preferable to select * unless you absolutely must have all information) and to not query more rows than you need to. You also normally wont want to use more than one query where one would suffice.

Also it might be of interest to note that you can capture the SQL generated by LINQ (see: http://msdn.microsoft.com/en-us/library/bb386961%28v=vs.110%29.aspx). Your query here looks really simple but you might be surprised at the stupid SQL that is sometimes generated by a LINQ to SQL statement. Updating the structure of the LINQ statement will affect the SQL that is generated which could affect the queries performance in the database. As I said though I don't think this is your problem but it's worth noting none the less.

Edit: Apparently I can't comment on Jesse C. Slicer's answer. I do like his solution because the query there is restricting the query results for each user to only the orders made by that user. So instead of getting all of the orders each time you only get the ones you want. This is an improvement but my solution above might be a bit better as it only hits the database once ... assuming of course that all of the results are able to fit into memory.

Answer 3

To make things short as I am on my mobile right now:

Calculate start- and enddate outside of the foreach loop. This is the most obvious step to performance here.

On a related note.. drop the hungarian Notation. Prefixing str and dt respectively adds no meaning to the name of the variable, especially when you aren't even using the String representation for anything but creating the datetime representation, which is beating around the bush from bottom to top...