I have very slow performances on EF. I have many foreach, mappings.

Do you have suggestion? I would like to make improvement without complicated refactoring and I would like to use EF.

  public bool Monitoring(MonitoringInformation monitoringInformation)
            if (null == monitoringInformation) throw new ArgumentNullException(nameof(monitoringInformation));

            bool insertSuccess = false;

            // false is for lazy loading = false
            using (var context = new MonitoringToolContext(false))
                var entity = MonitoringStatuses(monitoringInformation, context);
                context.Configuration.AutoDetectChangesEnabled = false;
                insertSuccess = true;
            return insertSuccess;

        private DataModels.MonitoringInformation MonitoringStatuses(MonitoringInformation monitoringInformation, MonitoringToolContext context)
            var retMi = new DataModels.MonitoringInformation
                InsertedDateTime = monitoringInformation.InsertedDateTime,
                Filename = monitoringInformation.Filename,
                GeneratedDate = monitoringInformation.GeneratedDate,
                Version = monitoringInformation.Version,
                FileType = monitoringInformation.FileType

            retMi.DeviceStatuses = DoDeviceStatus(monitoringInformation.Devices, context);

            retMi.HardwareStatusInformations = DoHardwareComponentStatuses(monitoringInformation.HardwareComponents, monitoringInformation.EODs, context);

            return retMi;

        private ICollection<DataModels.DeviceStatus> DoDeviceStatus(IEnumerable<Device> devices, MonitoringToolContext context)
            var dStatusList = new List<DataModels.DeviceStatus>();

            foreach (var device in devices)
                var tmp = context.Device.FirstOrDefault(x => x.Name == device.Name);
                if (tmp == null) continue;

                dStatusList.Add(new DataModels.DeviceStatus
                    DeviceId = tmp.DeviceId,
                    File = device.DeviceStatus.File,
                    Status = device.DeviceStatus.Status

            return dStatusList;

        private ICollection<DataModels.HardwareStatusInformation> DoHardwareComponentStatuses(IEnumerable<HardwareComponent> hwComponents, List<Eod> eods, MonitoringToolContext context)
            var dHwStatuses = new List<DataModels.HardwareStatusInformation>();

            foreach (var hardwarecomponent in hwComponents)

                var dHwComp = new DataModels.HardwareComponent();
                var dHwStatus = new DataModels.HardwareStatusInformation()
                    HardwareComponent = dHwComp,
                    DiskFree = hardwarecomponent.HardwareStatusInformation.DiskFree,
                    DiskTotal = hardwarecomponent.HardwareStatusInformation.DiskTotal,
                    LastUpdate = hardwarecomponent.HardwareStatusInformation.LastUpdate,
                    RamFree = hardwarecomponent.HardwareStatusInformation.RamFree,
                    RamTotal = hardwarecomponent.HardwareStatusInformation.RamTotal,
                    RamUnit = hardwarecomponent.HardwareStatusInformation.RamUnit,
                    UpTime = hardwarecomponent.HardwareStatusInformation.UpTime,
                    GroupId = hardwarecomponent.GroupId

                foreach (var softwarecomponent in hardwarecomponent.SoftwareComponents)
                    var tmpSwComp = context.SoftwareComponent.FirstOrDefault(x => x.Name == softwarecomponent.Name);
                    if (tmpSwComp == null) continue;

                    dHwStatus.HardwareComponent.SoftwareComponentStatus.Add(new DataModels.SoftwareComponentStatus()
                        SoftwareComponentId = tmpSwComp.SoftwareComponentId,
                        LastUpdate = softwarecomponent.SoftwareComponentStatus.LastUpdate,
                        Status = softwarecomponent.SoftwareComponentStatus.Status,
                        Version = softwarecomponent.SoftwareComponentStatus.Version,

                dHwComp.EodStatus = DoEodStatus(eods, context);

            return dHwStatuses;

        private ICollection<DataModels.EODStatus> DoEodStatus(IEnumerable<Eod> eods, MonitoringToolContext context)
            var dEodList = new List<DataModels.EODStatus>();

            foreach (var eod in eods)
                var tmpEod = context.EOD.FirstOrDefault(x => x.FileVersion == eod.FileVersion);
                if (tmpEod != null)
                    dEodList.Add(new DataModels.EODStatus()
                        EodId = tmpEod.EODId,
                        Location = eod.EodStatus.Location,
                        VersionType = eod.EodStatus.VersionType,
                        FileType = eod.EodStatus.FileType,
                        FileVersion = eod.EodStatus.FileVersion,
                        EffectiveDate = eod.EodStatus.EffectiveDate

            return dEodList;
  • 4
    \$\begingroup\$ You need to share more detials. What does slow mean? How slow is it? How much data do you process? What's the schema? \$\endgroup\$
    – t3chb0t
    Apr 3 '18 at 8:39

While there's definitely a lack of concrete numbers, the simple fact that you loop through several collections and do FirstOrDefault for each element is undoubtedly a major performance issue.

You should do a single query and store the results in a Dictionary<T, T>, and then while looping fetch the necessary data from the dictionary (using TryGetValue).

If you'd started a SQL Server Profiler session you would have no doubt seen hundreds of SELECT queries; replacing those with a single one would already alleviate much of your server load.

Now, some pointers WRT the code:

  • MonitoringStatuses isn't a proper method name and doesn't even return what you say it does.
  • DoHardwareComponentStatuses, DoDeviceStatus and DoEodStatus aren't great method names either.
  • Passing around MonitoringToolContext context to each method seems cumbersome to me. Move MonitoringStatuses and its dependent methods to a separate class of its own -- e.g. MonitoringInformationCreator -- and pass the MonitoringToolContext to its constructor, thus storing it at class level.
  • Do not pointlessly abbreviate: retMi isn't a useful variable name, dHwComp and dHwStatus even less so (what is the "d" even referring to?).
  • While we're on the topic of bad variable names: tmp is extremely bad. tmpEod isn't much better.
  • Do not use words like "List" in variable names, e.g. dStatusList. Just use the plural of what the collection contains, e.g. var deviceStatuses = new List<DataModels.DeviceStatus>();.
  • If you have an abbreviation that is longer than two letters, e.g. EOD, then the C# code style rulebook says to use PascalCase and thus name it Eod. C# code should never have more than three capital letters in a row, and even having three in a row is an exceptional case. So EODStatus should be renamed (you do apply this correctly when naming the property: EodStatus).

A lot of your code seems to involve copying the contents of one object to another. Perhaps using something like AutoMapper could make this easier for you?


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