Link to home
Start Free TrialLog in
Avatar of programmerist 1983
programmerist 1983Flag for Türkiye

asked on

Can you help me to gear up my linq query for better performance?

I have really big performance issue with my linq query. I need your help to solve this problem . First of all, I don't have any correct INDEX issue. I checked them all via appinsight. Can you give me some advise to gear up my linq query because it takes 8 seconds or more. How can I improve the performance of my slow linq query.

   string sortColumnName = GetSortColumnName(request);

            if (!string.IsNullOrEmpty(sortColumnName))
            {
                queryOrdered = queryOrdered.OrderBy(sortColumnName);
            }

            var data = await queryOrdered.Skip(request.Start).Take(request.Length).ToListAsync().ConfigureAwait(false);

            data.ForEach(x =>
            {
                foreach (var item in x.SalesItems)
                {
                    if (flightSalesItemType.Any(y => y == item.Value))
                    {
                        var flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).FirstOrDefault(q => q.SalesItemId == item.Key);
                        if (flightSales != null)
                        {
                            var flightLeg = DbContext.Set<FlightLeg>().Include(q => q.Airline).FirstOrDefault(q => q.PNRId == flightSales.PNRID);

                            x.TicketNo = flightSales.TicketNumber;
                            x.ServiceFee = flightSales.FlightTicketFare.Sum(t => t.ServiceFee);
                            x.PassengerName = flightSales.FlightPassenger.Name + " " + flightSales.FlightPassenger.Surname;
                            x.AirlineProviderName = flightLeg.Airline.Code;
                            x.Route = flightLeg.OriginCityCode + " - " + flightLeg.DestinationCityCode;
                        }
                    }
                }
            });

Open in new window

Avatar of AndyAinscow
AndyAinscow
Flag of Switzerland image

First step is to analyse what that does then decide is all of that a) necessary b) done so as not to keep repeating the same step to deliver the same information.

ps.  Just because you can use linq to do something does not mean it is appropriate technology to actually do that task.
Hi,
See my answer to your previous question. I'll paste it here, too, for convenience:

SQL Server Profiler is a tool available on desktop/on-prem version of SQL Server Management Studio. By using this tool, you'll be able to see the query that it's executed based on the EF code transformation. Also, from SQL Server Management Studio, on the query execution, you're able to see the execution plan and see where are the most expensive parts.
I would be creating a stored procedure in SQL Server (using T-SQL) and then using LINQ to execute that stored procedure (by naming the SP as a datasource)

That way you can test and optimise the SP within SQL Server.... It becomes a single call to SQL from LINQ.

For a simple example (also incorporating a web ui) have a look at : https://www.c-sharpcorner.com/article/stored-procedure-with-linq-to-sql/

In particular, note the C# code snippets more so than the Web ui (not sure of your environment).
Well, normally you would try to place the expensive calls not in the loop. So you should look into your data model, whether it is possible to move the DbContext calls out of the loops body.

Also what kind of performance problem do you have and how do you measure it?
When performance matters, why is your method async?
Avatar of programmerist 1983

ASKER

"Well, normally you would try to place the expensive calls not in the loop." can you modify my query to teach me better. Thank you for your great help.
While crafting an explanation, I've stumbled upon this (core loop and relevant body):

data.ForEach(x =>
{
  foreach (var item in x.SalesItems)
  {
    var flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).FirstOrDefault(q => q.SalesItemId == item.Key);
    var flightLeg = DbContext.Set<FlightLeg>().Include(q => q.Airline).FirstOrDefault(q => q.PNRId == flightSales.PNRID);
    x.TicketNo = flightSales.TicketNumber;
  }
});

Open in new window

You're assigned to x the values more than one time in the worst case. This makes no sense.

It should look like this:

data.ForEach(x =>
{
  var flightSales;
  foreach (var item in x.SalesItems)
  {
    flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).FirstOrDefault(q => q.SalesItemId == item.Key);
  }
  
  if (flightSales != null)
  {  
    var flightLeg = DbContext.Set<FlightLeg>().Include(q => q.Airline).FirstOrDefault(q => q.PNRId == flightSales.PNRID);
    x.TicketNo = flightSales.TicketNumber;
  }
});

Open in new window

And now you can move the DbContext calls:

data.ForEach(x =>
{
  var itemKeys = new List<typeof(item.Key)>();
  foreach (var item in x.SalesItems)
  {
    if (flightSalesItemType.Any(y => y == item.Value))
    {
      itemKeys.Add(tem.Key);
    }
  }
  
  var flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).FirstOrDefault(q => q.SalesItemId == item.Key); // use itemKeys here.  
  if (flightSales != null)
  {  
    var flightLeg = DbContext.Set<FlightLeg>().Include(q => q.Airline).FirstOrDefault(q => q.PNRId == flightSales.PNRID);
    x.TicketNo = flightSales.TicketNumber;
  }
});

Open in new window


Overall: the second sample shows that you need/want one flightSales. Thus you should review the way you loop over the items and find your flights.
I looks like it should be simpler, at least line 3-10 in the third example can be changed to a single LinQ expression.
Hi ste5an ;

First of all , I want to say my  appreciation for your great help. But When I use above method  my query getting slower. Maybe I need indexof instead of contains.
var flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).FirstOrDefault(q => items.Key.Contains(q.SalesItemId)); 

Open in new window


But I realized that new things. When you take a look at my below code :

 sw = Stopwatch.StartNew();
            var data = queryOrdered.AsParallel().AsQueryable().AsNoTracking()
                .Skip(request.Start)
                .Take(request.Length)
                .OrderBy(sortColumnName)
                .ToList();

Open in new window


Skip take takes time. I don't know how to handle it. I am using .netcore 2.2.
Well, hard to tell. Post the entire (concise, but complete) original code and the modified version.

But: Just look further at the code outline:

data.ForEach(x =>
{
  var itemKeys = new List<typeof(item.Key)>();
  foreach (var item in x.SalesItems)
  {
    if (flightSalesItemType.Any(y => y == item.Value))
    {
      itemKeys.Add(tem.Key);
    }
  }
  
  var flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).FirstOrDefault(q => q.SalesItemId == item.Key); // use itemKeys here.  
  if (flightSales != null)
  {  
    var flightLeg = DbContext.Set<FlightLeg>().Include(q => q.Airline).FirstOrDefault(q => q.PNRId == flightSales.PNRID);
    x.TicketNo = flightSales.TicketNumber;
  }
});

Open in new window

At line 12 and line 15 you have an invariant part, which is not depended in the loop (x):

var flightSales = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare).
var flightLeg = DbContext.Set<FlightLeg>().Include(q => q.Airline).

Open in new window


Here I would simply test extracting this:

var flightSalesBase = DbContext.Set<FlightTicketNumber>().Include(q => q.FlightPassenger).Include(q => q.FlightTicketFare);
var flightLegBase = DbContext.Set<FlightLeg>().Include(q => q.Airline);
data.ForEach(x =>
{
  var itemKeys = new List<typeof(item.Key)>();
  foreach (var item in x.SalesItems)
  {
    if (flightSalesItemType.Any(y => y == item.Value))
    {
      itemKeys.Add(tem.Key);
    }
  }

  var flightSales = flightSalesBase.FirstOrDefault(q => q.SalesItemId == item.Key); // use itemKeys here.
  if (flightSales != null)
  {
    var flightLeg = flightLegBase.FirstOrDefault(q => q.PNRId == flightSales.PNRID);
    x.TicketNo = flightSales.TicketNumber;
  }
});

Open in new window

This may have no effect, as EF is pretty optimized. Or may be even negative, when it comes to a scenario where it loads all objects, instead of the necessary.
But there is a slightly chance that may help, cause in a normal scenario, with a small amount of objects, there should be call overhead.
Hi,

I would suggest to let out the includes from the queries. EF will take care to issue the required queries when needed. Check how your query performs in this scenario.
Also, as I have suggested, try to use SQL Server Profiler in order to get the query that is executed on sql server
Hi @Ste5an ;

Thank you so much your adorable help but "var itemKeys = new List<typeof(item.Key)>();" never works because of item outside of the foreach.On the other hand, I don't understand "// use itemKeys here." But how? can you explain on it.
Line 5 in my last example:
You must specify the type of item.Keys. As you have only posted snippets I don't know it, thus the hint with "typeof(items.Keys)".

Line 10 in my last example:
Compare it with your original code. The idea is not the call DbContext in the inner loop. But create a list of values which is used later in a single call. This what the comment "// use itemKeys here." means. Adjust the LINQ query at this position to use the itemKeys list.
Seems you are creating a new "select" each and every time and not really giving LINQ a chance of utilising a dataset as such....

could be using ThenInclude to retrieve the detail rows... see bad example below.

But, before anything else, would love to see the data model and how those tables hang together....

With a VIEW or a Stored Procedure, you could be retrieving the full (ie joined tables) to make it quicker and (arguably) easier to retrieve details.

Any objection to using T-SQL to help your LINQ query ?

Now, that bad (in terms of relevance) example.... from https://entityframeworkcore.com/querying-data-include-theninclude 

Say we want to get all Invoices for a Customer. The code would look something like :
using (var context = new MyContext())
{
    var customers = context.Customers
        .Include(c => c.Invoices)
        .ToList();
}

Open in new window

Using the method ThenInclude() you can then access any *related* data. In this example, getting the Invoice Item Lines attached to each Invoice....

using (var context = new MyContext())
{
    var customers = context.Customers
        .Include(i => i.Invoices)
            .ThenInclude(it => it.Items)
        .ToList();
}

Open in new window

Still, would like to see data models, and LINQ is not my area of expertise, but have played with it a little bit.... So, I would naturally tend towards a Stored Procedure type approach in any case.
Hello again;

I modified my query but I need your check What do you think of it?  

 datalist.AsParallel().ToList().ForEach(x =>
            {
                var itemKeys = new List<dynamic>();
                foreach (var item in x.SalesItems)
                {
                    if (flightSalesItemType.Any(y => y == item.Value))
                    {
                        itemKeys.Add(item.Key);
                    }
                }

                var flightSales = flightSalesBase.FirstOrDefault(q => itemKeys.IndexOf(q.SalesItemId)>0);
                if (flightSales != null)
                {
                    var flightLeg = flightLegBase.FirstOrDefault(q => q.PNRId == flightSales.PNRID);
                    x.TicketNo = flightSales.TicketNumber;
                }

            });

Open in new window

Hi; What  do you think of my above answer?
Just test it..
This question needs an answer!
Become an EE member today
7 DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform.
View membership options
or
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.