1

I have to insert multiple relations and having issues with the Context.SaveChanges action which takes like forever to complete. I already tried multiple ways to add these entities to database but nothing seems to help me out.

My models are build in the following way:

public class Agreement : GdSoftDeleteEntity
{
    public DateTime Date { get; set; }
    public AgreementType AgreementType { get; set; }

    public virtual ICollection<PersonAgreementRelation> PersonAgreementRelations { get; set; }
    public virtual ICollection<ImageSearchAppointment> ImageSearchAppointments { get; set; }
}

public class Person : GdSoftDeleteEntity
{
    public string Name { get; set; }
    public string FirstName { get; set; }

    // E-mail is in identityuser
    //public string EmailAddress { get; set; }

    public virtual PersonType PersonType { get; set; }

    public virtual ICollection<PersonAgreementRelation> PersonAgreementRelations { get; set; }
    public virtual ICollection<PersonPersonRelation> PersonMasters { get; set; }
    public virtual ICollection<PersonPersonRelation> PersonSlaves { get; set; }
}

public class PersonAgreementRelation : GdSoftDeleteEntity
{
    public int PersonId { get; set; }
    public virtual Person Person { get; set; }

    public int AgreementId { get; set; }
    public virtual Agreement Agreement { get; set; }

    public virtual PersonAgreementRole PersonAgreementRole { get; set; }
}

public class ImageSearchAppointment : GdSoftDeleteEntity
{
    public string Name { get; set; }
    public bool ShowResultsToCustomer { get; set; }
    public bool HasImageFeed { get; set; }
    public int AgreementId { get; set; }
    public virtual Agreement Agreement { get; set; }

    public Periodicity Periodicity { get; set; }
    public PeriodicityCategory PeriodicityCategory { get; set; }

    public virtual ICollection<ImageSearchCommand> ImageSearchCommands { get; set; }
    public virtual ICollection<ImageSearchAppointmentWebDomainWhitelist> ImageSearchAppointmentWebDomainWhitelists { get; set; }
    public virtual ICollection<ImageSearchAppointmentWebDomainExtension> ImageSearchAppointmentWebDomainExtensions { get; set; }
}

public class ImageSearchCommand : GdSoftDeleteEntity
{
    public int ImageSearchAppointmentId { get; set; }
    public virtual ImageSearchAppointment ImageSearchAppointment { get; set; }

    public int? ImageSearchAppointmentCredentialsId { get; set; }
    public virtual ImageSearchAppointmentCredentials ImageSearchAppointmentCredentials { get; set; }

    public DateTime Date { get; set; }

    //public bool Invoiced { get; set; }
    public int NumberOfImages { get; set; }
    public DateTime ImageCollectionProcessedDate { get; set; }

    public virtual ICollection<ImageSearchExecution> ImageSearchExecutions { get; set; }
}

In my service, I have written following code:

public int AddAgreement(int personId, AgreementDto agreementDto)
        {
            Context.Configuration.LazyLoadingEnabled = false;
            //var person = Context.Persons.SingleOrDefault(el => el.Id == personId);
            var person = Context.Persons
                .SingleOrDefault(x => x.Id == personId);
            if (person == null)
            {
                throw new GraphicsDetectiveInvalidDataTypeException($"No person found for Id: {personId}");
            }

            if (agreementDto == null)
            {
                throw new GraphicsDetectiveInvalidDataTypeException("Invalid agreementDto");
            }

            //TODO: Check if OKAY!!!

            if (agreementDto.ImageSearchAppointmentDto.Count == 0)
            {
                throw new GraphicsDetectiveInvalidDataTypeException("Count of imagesearchappointments can't be lower than 0");
            }

            //set agreement properties
            var agreement = new Agreement
            {
                Date = agreementDto.DateTime,
                AgreementType = AgreementType.WwwImageSearch,
                //ImageSearchAppointments = new List<ImageSearchAppointment>(),
                //IsDeleted = false
            };
            Context.Agreements.Add(agreement);
            Context.SaveChanges();

            //var personAdminId = Context.Users.Single(x => x.Email == ConfigurationManager.AppSettings["DefaultGdAdminEmail"]).PersonId;
            // Dit werkt niet. Moet in 2 stappen
            //set personagreementrelations for new agreement
            var adminEmail = ConfigurationManager.AppSettings["DefaultGdAdminEmail"];    
            var personAdminId = Context.Users
                .SingleOrDefault(x => x.Email == adminEmail)
                .PersonId;

            var personPmId = Context.Persons.Single(x => x.Name == "My name").Id;
            var personAgreementRelations = new List<PersonAgreementRelation>()
                {
                    new PersonAgreementRelation
                    {
                        AgreementId = agreement.Id,
                        PersonId = personId,
                        PersonAgreementRole = PersonAgreementRole.Client,
                    },
                    new PersonAgreementRelation
                    {
                        AgreementId = agreement.Id,
                        PersonAgreementRole = PersonAgreementRole.Supplier,
                        PersonId = personPmId,
                    },
                     new PersonAgreementRelation
                    {
                        AgreementId = agreement.Id,
                        PersonAgreementRole = PersonAgreementRole.Admin,
                        PersonId = personAdminId,
                    }
                };
            foreach (var personAgreementRelation in personAgreementRelations)
            {
                Context.PersonAgreementRelations.Add(personAgreementRelation);
            }

            Context.Configuration.ValidateOnSaveEnabled = false;
            Context.Configuration.AutoDetectChangesEnabled = false;

            Context.SaveChanges();

            Context.Configuration.ValidateOnSaveEnabled = true;
            Context.Configuration.AutoDetectChangesEnabled = true;
            Context.Configuration.LazyLoadingEnabled = true;

            return agreement.Id;
        }

        public void AddFirstImageSearchAppointmentToAgreement(int agreementId, ImageSearchAppointmentDto imageSearchAppointmentDto)
        {
            Context.Configuration.LazyLoadingEnabled = false;
            var agreement = Context.Agreements.SingleOrDefault(x => x.Id == agreementId);
            if (agreement == null)
            {
                throw new GraphicsDetectiveInvalidDataTypeException($"No agreement found for id {agreementId}");
            }
            var appointmentType = imageSearchAppointmentDto;
            if (appointmentType == null)
            {
                throw new GraphicsDetectiveInvalidDataTypeException($"No valid imageSearchAppointment");
            }
            if (appointmentType.ImageSearchCommandDto.Count == 0)
            {
                throw new GraphicsDetectiveInvalidDataTypeException("No imageSearchCommand");
            }

            var imageSearchAppointment = new ImageSearchAppointment
            {
                AgreementId = agreement.Id,
                Agreement = agreement,
                Name = appointmentType.Name,
                Periodicity = appointmentType.Periodicity,
                PeriodicityCategory = appointmentType.PeriodicityCategory,
                ShowResultsToCustomer = appointmentType.ShowResultsToCustomer,
                ImageSearchAppointmentWebDomainExtensions = new List<ImageSearchAppointmentWebDomainExtension>(),
                ImageSearchCommands = new List<ImageSearchCommand>(),
                ImageSearchAppointmentWebDomainWhitelists = new List<ImageSearchAppointmentWebDomainWhitelist>(),
                IsDeleted = false
            };

            var imageSearchCommandDto = appointmentType.ImageSearchCommandDto.Single();
            var imageSearchCommand = new ImageSearchCommand()
            {
                ImageSearchAppointment = imageSearchAppointment,
                Date = imageSearchCommandDto.Date,
                NumberOfImages = imageSearchCommandDto.NumberOfImages,
                ImageCollectionProcessedDate = imageSearchCommandDto.ImageCollectionProcessedDate,
                IsDeleted = false
            };

            if (imageSearchCommandDto.ImageSearchAppointmentCredentialsDto != null)
            {
                imageSearchCommand.ImageSearchAppointmentCredentials = new ImageSearchAppointmentCredentials
                {
                    FtpProfileType = imageSearchCommandDto.ImageSearchAppointmentCredentialsDto.FtpProfileType,
                    Location = imageSearchCommandDto.ImageSearchAppointmentCredentialsDto.Location,
                    Username = imageSearchCommandDto.ImageSearchAppointmentCredentialsDto.Username,
                    Password = imageSearchCommandDto.ImageSearchAppointmentCredentialsDto.Password,
                    UsePassive = imageSearchCommandDto.ImageSearchAppointmentCredentialsDto.UsePassive,
                    IsDeleted = false
                };
            }
            imageSearchAppointment.ImageSearchCommands.Add(imageSearchCommand);

            if (!imageSearchAppointment.ShowResultsToCustomer)
            {
                var webDomainExtensions = appointmentType.WebDomainExtensionDtos
                    .Select(x => new ImageSearchAppointmentWebDomainExtension()
                    {
                        ImageSearchAppointment = imageSearchAppointment,
                        WebDomainExtensionId = x.Id
                    })
                    .ToList();

                imageSearchAppointment.ImageSearchAppointmentWebDomainExtensions = webDomainExtensions;
            }

            Context.ImageSearchAppointments.Add(imageSearchAppointment);
            Context.SaveChanges();

            Context.Configuration.LazyLoadingEnabled = true;
        }

I used dotTrace to profile these functions and it takes about 9 minutes to add the new entities to my database.

The database is an Azure SQL database, tier S3

I tried the proposed solution and adapted my code as follow:

public int AddAgreement(int personId, AgreementDto agreementDto)
        {
            var agreementId = 0;
            using (var context = new GdDbContext())
            {
                GdDbConfiguration.SuspendExecutionStrategy = true;
                context.Configuration.LazyLoadingEnabled = true;
                //var person = Context.Persons.SingleOrDefault(el => el.Id == personId);
                var person = context.Persons
                    .SingleOrDefault(x => x.Id == personId);
                if (person == null)
                {
                    throw new GraphicsDetectiveInvalidDataTypeException($"No person found for Id: {personId}");
                }

                //var personAdminId = Context.Users.Single(x => x.Email == ConfigurationManager.AppSettings["DefaultGdAdminEmail"]).PersonId;
                // Dit werkt niet. Moet in 2 stappen
                //set personagreementrelations for new agreement
                var adminEmail = ConfigurationManager.AppSettings["DefaultGdAdminEmail"];
                var personAdminId = context.Users
                    .Where(x => x.Email == adminEmail)
                    .Include(x => x.Person)
                    .First()
                    .Person.Id;

                var personPmId = context.Persons.First(x => x.Name == "My name").Id;

                using (var dbContextTransaction = context.Database.BeginTransaction())
                {
                    try
                    {

                        if (agreementDto == null)
                        {
                            throw new GraphicsDetectiveInvalidDataTypeException("Invalid agreementDto");
                        }

                        //TODO: Check if OKAY!!!

                        if (agreementDto.ImageSearchAppointmentDto.Count == 0)
                        {
                            throw new GraphicsDetectiveInvalidDataTypeException("Count of imagesearchappointments can't be lower than 0");
                        }

                        //set agreement properties
                        var agreement = new Agreement
                        {
                            Date = agreementDto.DateTime,
                            AgreementType = AgreementType.WwwImageSearch,
                            //ImageSearchAppointments = new List<ImageSearchAppointment>(),
                            //IsDeleted = false
                        };
                        context.Agreements.Add(agreement);
                        //Context.SaveChanges();

                        var personAgreementRelations = new List<PersonAgreementRelation>()
                        {
                            new PersonAgreementRelation
                            {
                                //Agreement = agreement,
                                AgreementId = agreement.Id,
                                PersonId = personId,
                                //Person = person,
                                PersonAgreementRole = PersonAgreementRole.Client,
                                //IsDeleted = false
                            },
                            new PersonAgreementRelation
                            {
                                //Agreement = agreement,
                                AgreementId = agreement.Id,
                                PersonAgreementRole = PersonAgreementRole.Supplier,
                                PersonId = personPmId,
                                //Person = personPm,
                                //IsDeleted = false
                            },
                             new PersonAgreementRelation
                            {
                                //Agreement = agreement,
                                AgreementId = agreement.Id,
                                PersonAgreementRole = PersonAgreementRole.Admin,
                                PersonId = personAdminId,
                                //Person = personAdmin,
                            }
                        };

                        foreach (var personAgreementRelation in personAgreementRelations)
                        {
                            context.PersonAgreementRelations.Add(personAgreementRelation);
                        }
                        //agreement.PersonAgreementRelations = personAgreementRelations;

                        //Context.Agreements.Add(agreement);

                        context.Configuration.ValidateOnSaveEnabled = false;
                        context.Configuration.AutoDetectChangesEnabled = false;

                        //await Context.SaveChangesAsync();

                        context.SaveChanges();
                        dbContextTransaction.Commit();
                        //await Task.Run(async () => await Context.SaveChangesAsync());

                        context.Configuration.ValidateOnSaveEnabled = true;
                        context.Configuration.AutoDetectChangesEnabled = true;
                        context.Configuration.LazyLoadingEnabled = false;

                        agreementId = agreement.Id;
                    }
                    catch (Exception ex)
                    {
                        dbContextTransaction.Rollback();
                        throw ex;
                    }
                }
                GdDbConfiguration.SuspendExecutionStrategy = false;
            }

            return agreementId;
        }

but it's taking as much time as before

rubenj
  • 118
  • 1
  • 6
  • which method do you have a problem ? you have shown 2 methods. – Sampath Nov 22 '16 at 08:21
  • both of them. As well the insert of the PersonAgreementRelation list as the insert of the ImageSearchAppointment is very slow – rubenj Nov 22 '16 at 08:26
  • Is it always slow or becomes slow over time? How many appointments are already in database? – Evk Nov 22 '16 at 08:32
  • When I run this method on a very small local database, it only takes half a second. In my development database, there are 66 imagesearchappointments and 203 personagreementrelations – rubenj Nov 22 '16 at 08:34
  • Is your Azure SQL database in the same region as your application? It's important that database servers have very low-latency connections to their consumers. – Dai Nov 22 '16 at 08:41
  • They are both hosted in West Europe – rubenj Nov 22 '16 at 08:56

2 Answers2

0

You can follow below mentioned suggestions to improve the performance of above methods.

  1. Use FirstOrDefault() instead of SingleOrDefault().FirstOrDefault() is the fastest method.

  2. I can see that you have used Context.SaveChanges() method number of times on the same method.That will degrade the performnce of the method.So you must avoid that.Instead of use Transactions.

Like this : EF Transactions

using (var context = new YourContext()) 
            { 
                using (var dbContextTransaction = context.Database.BeginTransaction()) 
                { 
                    try 
                    { 
                        // your operations here

                        context.SaveChanges(); //this called only once
                        dbContextTransaction.Commit(); 
                    } 
                    catch (Exception) 
                    { 
                        dbContextTransaction.Rollback(); 
                    } 
                } 
            } 
  1. You can think about the implementaion of stored procedure if above will not give the enough improvement.
Sampath
  • 63,341
  • 64
  • 307
  • 441
  • Then I get following error: The configured execution strategy 'SqlAzureExecutionStrategy' does not support user initiated transactions. See http://go.microsoft.com/fwlink/?LinkId=309381 for additional information. – rubenj Nov 22 '16 at 09:24
  • hope you have all the workarounds on the above link.do you have any question about the workarounds ? – Sampath Nov 22 '16 at 09:42
  • I followed the workaround where the public static bool is set. You can see my new code in my edited answer (don't know if that's the right way on stackoverflow, it's my first question) – rubenj Nov 22 '16 at 10:06
  • still I can see `SingleOrDefault()` on your code.Why ? – Sampath Nov 22 '16 at 10:15
  • I forgot to change that one but when I'm debugging my code, I clearly notice that is not the reason why these methods take so long – rubenj Nov 22 '16 at 10:22
  • on local, you cannot see that.But on production those changes will give performance improvements. – Sampath Nov 22 '16 at 10:28
  • I'm testing against my development database in Azure – rubenj Nov 22 '16 at 10:29
  • you cannot test performance of `debug` mode.You have to test on `Release` mode. – Sampath Nov 22 '16 at 10:30
  • Ok, you're right. But onfurtunately your solution isn't working. It's still taking minutes to complete – rubenj Nov 22 '16 at 10:39
  • hope it's better than the earlier one ? – Sampath Nov 22 '16 at 10:40
  • Not really, I'm using an object from StopWatch class to track the ellapsed time, and it's saying about 10minutes. If I run the query generated by Entity Framework against the database, it's taking me about 10 seconds – rubenj Nov 22 '16 at 10:41
0

There are some performance issues with your code

Add Performance

foreach (var personAgreementRelation in personAgreementRelations)
{
    Context.PersonAgreementRelations.Add(personAgreementRelation);
}


Context.Configuration.ValidateOnSaveEnabled = false;
Context.Configuration.AutoDetectChangesEnabled = false;

You add multiple entities then disabled AutoDetectChanges. You normally do the inverse

Depending on the number of entities in your context, it can severely hurt your performance

In the method "AddFirstImageSearchAppointmentToAgreement", it seems you use an outside context which can be very bad if it contains already multiple thousands of entities.

See: Improve Entity Framework Add Performance

Badly used, adding an entity to the context with the Add method take more time than saving it in the database!

SaveChanges vs. Bulk Insert vs. BulkSaveChanges

SaveChanges is very slow. For every record to save, a database round-trip is required. This is particularly the case for SQL Azure user because of the extra latency.

Some library allows you to perform Bulk Insert

See:

Disclaimer: I'm the owner of the project Entity Framework Extensions

This library has a BulkSaveChanges features. It works exactly like SaveChanges but WAY FASTER!

// Context.SaveChanges();
Context.BulkSaveChanges();

EDIT: ADD additional information #1

I pasted my new code in Pastebin: link

Transaction

Why starting a transaction when you select your data and add entities to your context? It simply a VERY bad use of a transaction.

A transaction must be started as late as possible. In since BulkSaveChanges is already executed within a transaction, there is no point to create it.

Async.Result

 var personAdminId = context.Users.FirstOrDefaultAsync(x => x.Email == adminEmail).Result.PersonId;

I don't understand why you are using an async method here...

  • In best case, you get similar performance as using non-async method
  • In worse case, you suffer from some performance issue with async method

Cache Item

var adminEmail = ConfigurationManager.AppSettings["DefaultGdAdminEmail"];
var personAdminId = context.Users.FirstOrDefaultAsync(x => x.Email == adminEmail).Result.PersonId;

I don't know how many time you call the AddAgreement method, but I doubt the admin will change.

So if you call it 10,000 times, you make 10,000 database round-trip to get the same exact value every time.

Create a static variable instead and get the value only once! You will for sure save a lot of time here

Here is how I normally handle static variable of this kind:

var personAdminId = My.UserAdmin.Id;

public static class My
{
    private static User _userAdmin;
    public static User UserAdmin
    {
        get
        {
            if (_userAdmin == null)
            {
                using (var context = new GdDbContext())
                {
                    var adminEmail = ConfigurationManager.AppSettings["DefaultGdAdminEmail"];
                    _userAdmin = context.Users.FirstOrDefault(x => x.Email == adminEmail);
                }
            }

            return _userAdmin;
        }
    }
}

LazyLoadingEnabled

In the first code, you have LazyLoadingEnabled to false but not in your Pastebin code,

Disabling LazyLoading can help a little bit since it will not create a proxy instance.

Take 10m instead of 9m

Let me know after removing the transaction and disabling again LazyLoading if the performance is a little bit better.

The next step will be to know some statistics:

  • Around how many time the AddAgreement method is invoked
  • Around how many persons do you have in your database
  • Around how many entities in average is Saved by the AddAgreement method

EDIT: ADD additional information #2

Currently, the only way to improve really the performance is by reducing the number of database round-trip.

I see you are still searching the personAdminId every time. You could save maybe 30s to 1 minute just here by caching this value somewhere like a static variable.

You still have not answered the three questions:

  • Around how many time the AddAgreement method is invoked
  • Around how many persons do you have in your database
  • Around how many entities in average is Saved by the AddAgreement method

The goal of theses questions is to understand what's slow!

By example, if you call the AddAgreement method 10,000 times and you only have 2000 persons in the database, you are probably better to cache in two dictionary theses 2000 persons to save 20,000 database round-trip (Saving one to two minutes?).

Community
  • 1
  • 1
Jonathan Magnan
  • 10,874
  • 2
  • 38
  • 60
  • How many times this method is called, depends on how many new customers we can close a deal with. So AddAgreement might be called a couple of times a week, but can also be called once in two weeks... There are about 150 persons in the database and this method should add 6 entities, sometimes 10 or more, depending on amount of checked webdomains, to the database so this shouldn't take this much time at all I forgot to change the FirstOrDefault async to normal – rubenj Nov 24 '16 at 05:49
  • [Pastebin](http://pastebin.com/882NUNhG) This is my new code. The method now takes 7.5 minutes so there's already a speed improvement. I will try to use the BulkInsert extension and see if that also speeds things up – rubenj Nov 24 '16 at 06:31