1
\$\begingroup\$

I am looking for suggestion to make my for loop iterate faster. With about 2000 rows of data. Below code is taking lot of time to run. So far I have followed various Stack Overflow answers. For example reducing db queries and using Bulk insert to improve EF performance. It just improved marginally.

List<VehicleApplication> vehicleApplicationList = new List<VehicleApplication>();
List<ProductVehicleApplication> productVehicleApplicationList = new List<ProductVehicleApplication>();

if (inventoryjson["InventoryItems"][0]["VehicleApplications"] != null)
{
    for (int i = 0; i < inventoryjson["InventoryItems"][0]["VehicleApplications"].Count(); i++)
    {
        //database vehDb = new database();
        VehicleApplication vehicleApplication = new VehicleApplication();
        vehicleApplication.CategoryName = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["CategoryName"].ToString();
        vehicleApplication.MakeName = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["MakeName"].ToString();
        vehicleApplication.MfrLabel = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["MfrLabel"].ToString();
        vehicleApplication.ModelName = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["ModelName"].ToString();
        vehicleApplication.Note = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["Note"].ToString();
        vehicleApplication.PartTypeName = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["PartTypeName"].ToString();
        vehicleApplication.PositionName = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["PositionName"].ToString();
        vehicleApplication.Qty = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["Qty"].ToString();
        vehicleApplication.SubCategoryName = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["SubCategoryName"].ToString();
        vehicleApplication.YearID = inventoryjson["InventoryItems"][0]["VehicleApplications"][i]["YearID"].ToString();

        var existingVehicleApplication = dbVehiclesList.Where(x => x.MakeName == vehicleApplication.MakeName && x.ModelName == vehicleApplication.ModelName
            && x.CategoryName == vehicleApplication.CategoryName
            && x.MfrLabel == vehicleApplication.MfrLabel
            && x.PartTypeName == vehicleApplication.PartTypeName
            && x.PositionName == vehicleApplication.PositionName
            && x.Qty == vehicleApplication.Qty
            && x.YearID == vehicleApplication.YearID
            && x.SubCategoryName == vehicleApplication.SubCategoryName
            ).SingleOrDefault();

        if (existingVehicleApplication == null && vehicleApplicationList.Any(x => x.MakeName == vehicleApplication.MakeName && x.ModelName == vehicleApplication.ModelName && x.CategoryName == vehicleApplication.CategoryName && x.MfrLabel == vehicleApplication.MfrLabel && x.PartTypeName == vehicleApplication.PartTypeName && x.PositionName == vehicleApplication.PositionName && x.Qty == vehicleApplication.Qty && x.YearID == vehicleApplication.YearID && x.SubCategoryName == vehicleApplication.SubCategoryName) == false)
        {
            vehicleApplicationList.Add(vehicleApplication);
        }
        else
        {
            vehicleApplication = dbObj.VehicleApplication.SingleOrDefault(
            x => x.MakeName == vehicleApplication.MakeName
            && x.ModelName == vehicleApplication.ModelName
            && x.CategoryName == vehicleApplication.CategoryName
            && x.MfrLabel == vehicleApplication.MfrLabel
            && x.PartTypeName == vehicleApplication.PartTypeName
            && x.PositionName == vehicleApplication.PositionName
            && x.Qty == vehicleApplication.Qty
            && x.SubCategoryName == vehicleApplication.SubCategoryName
            && x.YearID == vehicleApplication.YearID);
        }

        ProductVehicleApplication productVehicleApplication = new ProductVehicleApplication();
        productVehicleApplication.Product = dbObj.Product.Single(x => x.Id == product.Id);
        productVehicleApplication.VehicleApplication = vehicleApplication;
        productVehicleApplicationList.Add(productVehicleApplication);
    }
}

dbObj.BulkInsert(productVehicleApplicationList);
dbObj.BulkInsert(vehicleApplicationList);
dbObj.SaveChanges();

UPDATE::1 From comments I have tried following. But it is still slow.

HashSet<VehicleApplication> vehicleApplicationList = new HashSet<VehicleApplication>();
                List<ProductVehicleApplication> productVehicleApplicationList = new List<ProductVehicleApplication>();
                if (jsonProduct["VehicleApplications"] != null)
                {
                 for (int i = 0; i < jsonProduct["VehicleApplications"].Count(); i++){
                            var vehicleApp = jsonProduct["VehicleApplications"][i];

                            VehicleApplication vehicleApplication = new VehicleApplication();
                            vehicleApplication.CategoryName = vehicleApp["CategoryName"].ToString();
                            vehicleApplication.MakeName = vehicleApp["MakeName"].ToString();
                            vehicleApplication.MfrLabel = vehicleApp["MfrLabel"].ToString();
                            vehicleApplication.ModelName = vehicleApp["ModelName"].ToString();
                            vehicleApplication.Note = vehicleApp["Note"].ToString();
                            vehicleApplication.PartTypeName = vehicleApp["PartTypeName"].ToString();
                            vehicleApplication.PositionName = vehicleApp["PositionName"].ToString();
                            vehicleApplication.Qty = vehicleApp["Qty"].ToString();
                            vehicleApplication.SubCategoryName = vehicleApp["SubCategoryName"].ToString();
                            vehicleApplication.YearID = vehicleApp["YearID"].ToString();


                            var existingVehicleApplication = dbVehiclesList.Where(x => x.MakeName == vehicleApplication.MakeName && x.ModelName == vehicleApplication.ModelName 
                                && x.CategoryName == vehicleApplication.CategoryName
                                && x.MfrLabel == vehicleApplication.MfrLabel
                                && x.PartTypeName == vehicleApplication.PartTypeName
                                && x.PositionName == vehicleApplication.PositionName
                                && x.Qty == vehicleApplication.Qty
                                && x.YearID == vehicleApplication.YearID
                                && x.SubCategoryName == vehicleApplication.SubCategoryName
                                ).SingleOrDefault(); 
                                
                    

                            if (existingVehicleApplication == null && vehicleApplicationList.Any(x => x.MakeName == vehicleApplication.MakeName && x.ModelName == vehicleApplication.ModelName && x.CategoryName == vehicleApplication.CategoryName && x.MfrLabel == vehicleApplication.MfrLabel && x.PartTypeName == vehicleApplication.PartTypeName && x.PositionName == vehicleApplication.PositionName && x.Qty == vehicleApplication.Qty && x.YearID == vehicleApplication.YearID && x.SubCategoryName == vehicleApplication.SubCategoryName) == false)
                            {
                                vehicleApplicationList.Add(vehicleApplication);
                            }
                            else
                            {
                                vehicleApplication = existingVehicleApplication;
                                //vehicleApplication = dbVehiclesList.SingleOrDefault(
                                //x => x.MakeName == vehicleApplication.MakeName
                                //&& x.ModelName == vehicleApplication.ModelName
                                //&& x.CategoryName == vehicleApplication.CategoryName
                                //&& x.MfrLabel == vehicleApplication.MfrLabel
                                //&& x.PartTypeName == vehicleApplication.PartTypeName
                                //&& x.PositionName == vehicleApplication.PositionName
                                //&& x.Qty == vehicleApplication.Qty
                                //&& x.SubCategoryName == vehicleApplication.SubCategoryName
                                //&& x.YearID == vehicleApplication.YearID);

                            }

                            ProductVehicleApplication productVehicleApplication = new ProductVehicleApplication();
                            productVehicleApplication.Product = dbProductList.Single(x=>x.Id==product.Id);
                            productVehicleApplication.VehicleApplication = vehicleApplication;


                            productVehicleApplicationList.Add(productVehicleApplication);
                        }
                    }

                    dbObj.BulkInsert(productVehicleApplicationList);
                    dbObj.BulkInsert(vehicleApplicationList);
                    dbObj.SaveChanges();
```
\$\endgroup\$
4
  • 3
    \$\begingroup\$ I changed the title so that it describes what I think it does, per site goals: "State what your code does in your title, not your main concerns about it.". Please check that I haven't misrepresented your code, and correct it if I have. The question is still lacking a lot of context needed for a decent review: the database schema and sample inputs, for starters. Please edit to add the necessary information. (Sorry I'm no expert in this language, so I'll still be unable to answer). \$\endgroup\$ Jul 10, 2021 at 8:57
  • \$\begingroup\$ Wow title make lot of sense now. My poor english :( \$\endgroup\$ Jul 10, 2021 at 9:00
  • \$\begingroup\$ Welcome to the Code Review Community. We are different than Stack Overflow, we need to see more of the code because we are trying to help you improve the code rather than debug whatever problem you are having. We need to see whole functions, and it would be better to see complete classes. Optimization is a case of finding bottlenecks in some cases. \$\endgroup\$
    – pacmaninbw
    Jul 10, 2021 at 13:00
  • 1
    \$\begingroup\$ Everything that could be done in the DB should be. Dump the "raw" objects to the DB and then do all the iteration and transformation. Probably require temp tables with pseudo-keys? In our case it went from 3+ hours of C#-cum-database to 15 seconds of a modestly complex stored procedure. Don't be afraid to create as many keys, compound-keys, unique columns as needed. All that up-front set up is nothing; any decent relational DB engine is light-speed faster than C#. Make them permanent tables if this process is done regularly. \$\endgroup\$
    – radarbob
    Jul 10, 2021 at 23:17

1 Answer 1

2
\$\begingroup\$

jsonProduct["VehicleApplications"].Count() should be outside the iteration, otherwise it will be executed on each iteration which impact the performance. So, you should do :

var count = jsonProduct["VehicleApplications"].Count();

for (int i = 0; i < count; i++)
{
    var vehicleApp = jsonProduct["VehicleApplications"][i];
    ///...... etc.
}

using SingleOrDefault is slower than FirstOrDefault and that's because of their behavior. SingleOrDefault will recheck the enumerable against the founded item, to see if there is any duplicates, while FirstOrDefault doesn't do this check. In your case, you don't need to check the enumerable, you need to get the first item that matches the condition. So, using FirstOrDefault would be better choice.

also, when you use HashSet you don't really need to recheck the set for duplicates when adding items, as the HashSet will take care of that, and will always store unique items. So, if you try to add an item that is already exists in the HashSet, it won't be added nor change the existing one.

try this version and see how it perform :

var vehicleApplications = jsonProduct["VehicleApplications"]; 

if(vehicleApplications != null)
{
    HashSet<VehicleApplication> vehicleApplicationHashSet = new HashSet<VehicleApplication>();
    
    List<ProductVehicleApplication> productVehicleApplicationList = new List<ProductVehicleApplication>();

    var count = vehicleApplications.Count();

    for (int i = 0; i < count; i++)
    {
        var vehicleApp = jsonProduct["VehicleApplications"][i];

        VehicleApplication vehicleApplication = new VehicleApplication
        {
            CategoryName = vehicleApp["CategoryName"].ToString(),
            MakeName = vehicleApp["MakeName"].ToString(),
            MfrLabel = vehicleApp["MfrLabel"].ToString(),
            ModelName = vehicleApp["ModelName"].ToString(),
            Note = vehicleApp["Note"].ToString(),
            PartTypeName = vehicleApp["PartTypeName"].ToString(),
            PositionName = vehicleApp["PositionName"].ToString(),
            Qty = vehicleApp["Qty"].ToString(),
            SubCategoryName = vehicleApp["SubCategoryName"].ToString(),
            YearID = vehicleApp["YearID"].ToString()
        };
        
        var existingVehicleApplication = dbVehiclesList.FirstOrDefault(x =>
               x.MakeName == vehicleApplication.MakeName 
            && x.ModelName == vehicleApplication.ModelName 
            && x.CategoryName == vehicleApplication.CategoryName
            && x.MfrLabel == vehicleApplication.MfrLabel
            && x.PartTypeName == vehicleApplication.PartTypeName
            && x.PositionName == vehicleApplication.PositionName
            && x.Qty == vehicleApplication.Qty
            && x.YearID == vehicleApplication.YearID
            && x.SubCategoryName == vehicleApplication.SubCategoryName); 
        
        if(existingVehicleApplication != null)
        {
            vehicleApplication = existingVehicleApplication;
        }
        else
        {
            // if the vehicleApp is already in the hashset, then it'll not be added.
            vehicleApplicationHashSet.Add(vehicleApplication);
        }
        
        ProductVehicleApplication productVehicleApplication = new ProductVehicleApplication
        {
            Product = dbProductList.FirstOrDefault(x=>x.Id==product.Id),
            VehicleApplication = vehicleApplication
        };
        
        productVehicleApplicationList.Add(productVehicleApplication);
    }

    dbObj.BulkInsert(productVehicleApplicationList);
    dbObj.BulkInsert(vehicleApplicationList);
    dbObj.SaveChanges();
}
\$\endgroup\$
2
  • \$\begingroup\$ It does help me a little improvement but var existingVehicleApplication = dbVehiclesList.FirstOrDefault Querying against database as dbVehiclesList is AsEnumerable dbProductList.FirstOrDefault same case here. Is that possible that I get data from these tables into a very fast variable so I dont have to db query? \$\endgroup\$ Jul 10, 2021 at 8:10
  • 2
    \$\begingroup\$ @JaskaranSingh what ORM are you using? because entity framework does not need t o cast as AsEnumerable .. \$\endgroup\$
    – iSR5
    Jul 10, 2021 at 12:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.