-3
\$\begingroup\$

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.MonitoringInformation.Add(entity);
                context.Configuration.AutoDetectChangesEnabled = false;
                context.SaveChanges();
                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);

                dHwStatuses.Add(dHwStatus);
            }
            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;
        }
\$\endgroup\$

closed as off-topic by t3chb0t, Toby Speight, Stephen Rauch, Sᴀᴍ Onᴇᴌᴀ, Imus Apr 3 '18 at 13:57

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – t3chb0t, Toby Speight, Stephen Rauch, Sᴀᴍ Onᴇᴌᴀ, Imus
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 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
1
\$\begingroup\$

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?

\$\endgroup\$

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