horrible and big LINQ statement optimisation

Question

I have to to a rather large request to a database to fetch a bunch of data, it's however taking a noticeable time to run. is there some way to increase the performance on this? preemptive apologies for the ugly code (I did have a version that segmented this into multiple smaller functions but that was even slower)

from contact in _database.OBJECTCONTACT
                where contact.OBJECTCONTACTOWNER.Any(o => o.OBJECTID == id && o.OBJECTTYPE == type) && contact.ACTIVE >= 1 && CheckViewAccess(contact)
                group contact by (contact.OBJECTCONTACTPROJECT.Any() ? contact.OBJECTCONTACTPROJECT.First().OBJECTPROJECT.PROJECTNAME : "General") into projectGroup
                select new ProjectViewModel()
                {
                    ProjectName = projectGroup.Key,
                    ContactGroups = (from g in _database.OBJECTGROUP
                            where g.GROUPTYPE == "CONTACT" && ContactsModule.CheckUserRole("View", g.OBJECTTYPE, g.GROUPNAME)
                                  select new ContactGroupViewModel()
                                  {
                                      CanEdit = ContactsModule.CheckUserRole("Edit", g.OBJECTTYPE, g.GROUPNAME),
                                      GroupId = g.OBJECTGROUPID,
                                      GroupName = g.GROUPNAME,
                                      Contacts = (from c in projectGroup
                                              join l in _database.OBJECTCONTACTLOCATION on c.OBJECTCONTACTLOCATIONID equals l.OBJECTCONTACTLOCATIONID into lgrp from loc in lgrp.DefaultIfEmpty(null)
                                              orderby c.NAME
                                              select new ContactViewModel()
                                              {
                                                  Id = (int)c.OBJECTCONTACTID,
                                                  Name = c.NAME,
                                                  Description = c.DESCRIPTION,
                                                  ContactInformation = CreateContactInfoViewmodels(c),
                                                  Owners = c.OBJECTCONTACTOWNER.Where(owner => owner.OBJECTTYPE == "AIRPORT")
                                                      .Select(owner => ContactOwnerViewModel.FromOwnerId(owner.OBJECTID, owner.OBJECTTYPE)).ToList(),
                                                  Projects = c.OBJECTCONTACTPROJECT.Select(proj => proj.OBJECTPROJECT).ToList(),
                                                  Typename = GetTypeName(c),
                                                  TypeId = c.OBJECTCONTACTTYPEID ?? 0,
                                                  ContactGroupId = c.OBJECTGROUPID,
                                                  ContactGroup = g.GROUPNAME,
                                                  Editable = CheckAccessBool("EDIT", c),
                                                  Location = loc != null ? new LocationViewModel()
                                                  {
                                                      Address = loc.ADDRESS,
                                                      GoogleMapLink = loc.GMAPADDRESS,
                                                      LocationId = loc.OBJECTCONTACTLOCATIONID,
                                                      LatLon = Tuple.Create(loc.LATITUDE, loc.LONGITUDE)
                                                  } : null,
                                              }).ToList()
                                  }).ToList()
                }).ToList();

I think I should be able to use joins to move the entire DB fetch code to the top (theoretically improving perfomance) but I am having trouble finding the syntax which would suit my needs

LINQ isn't a replacement for SQL. You *wouldn't* need any of those joins if you created entities with navigation properties and collections pointing to each other. You could load one object and retrieve an entire graph. — Panagiotis Kanavos, Jul 26 '18 at 11:45
Apart from this your questions is actually a code-review which is better suited at codereview.stackexchange.com Apart from this you should definitly use a profiler to indicate what parts of that code actually are impacting your performance. — MakePeaceGreatAgain, Jul 26 '18 at 11:46
LINQ *to the database* isn't the place to map the Model entities to your ViewModels either. Do that *after* you load the data. I'm surprised that this even runs, as many operations can't be converted to SQL. Most likely something loads entire tables into memory so they can be processed by LINQ to Objects instead of LINQ to EF — Panagiotis Kanavos, Jul 26 '18 at 11:47
You should capture the generated SQL and try to see if you can make it more performant. Also consider profiling it in SSMS to see if there are indexes you can add to your DB to make the query run faster. — juharr, Jul 26 '18 at 11:47
I guess this is ef-core because the query is speckled with code that's executed client-side, which necessarily leads to considerable amounts of data that must be read. — Gert Arnold, Jul 26 '18 at 11:48
There's a reason Models are separate from ViewModels in MVVM and the name is Model, not Table. The classes you need to display one *view* most of the time are *not* suitable for business logic. One ViewModel may contain data from multiple entities, flattened to lists for display on grids, aggregated for display on labels etc. Mixing them up makes the code a *lot* harder to maintain. In this case there are VMs, Models and tables all mixed together — Panagiotis Kanavos, Jul 26 '18 at 11:50
@GertArnold That is one disadvantage I think to the EF Core model versus the EF / LINQ to SQL model that throws an error for untranslatable queries, it is violating the [Principle of Least Suprise](https://en.wikipedia.org/wiki/Principle_of_least_astonishment). — NetMage, Jul 26 '18 at 17:56
@NetMage I fully agree. I wish ef-core didn't auto-switch to client-side evaluation by default, but only deliberately. BTW, LINQ-to-SQL also auto-switched to client-side evaluation, without the option to turn it off, as ef-core has. — Gert Arnold, Jul 26 '18 at 18:00
@GertArnold At least on .Net 4.7, I don't see that happening with LINQ to SQL. — NetMage, Jul 26 '18 at 18:07
@NetMage Maybe not in al cases. I'm sure there were cases where it did, but it's long ago... — Gert Arnold, Jul 26 '18 at 18:09
Something feels fishy with this query. _database.OBJECTCONTACT, is this a DbSet? What doesn't fit is "&& CheckViewAccess(contact)" & Tuple.Create(..) If those were fed to Linq2EF that would have spit the dummy.. I'd be very suspicious that _database is returning IEnumerable/List and then follow up queries are triggering lazy loads against Linq2Obj. This would show up easily in a profiler. To get to the bottom of this you should look at the table structure and then how your entities can be related through references without relying or declaring explicit joins in every Linq query. — Steve Py, Jul 26 '18 at 22:08

score 0 · Answer 1 · answered Jul 27 '18 at 11:35

Thanks everyone for coming with suggestions. I am in a situation where I'm not able to do much with the database itself so I'm making the best of what I have. my hands a bit tied in regards to the tools at my disposal (also fairly old codebase I think it's EF 5 or something like that)

this version moves the DB transaction to the top (so that is fewer fetches) and does a lot of data manipulation at the bottom.

// general object is created above
var res = (from contact in _database.OBJECTCONTACT.AsEnumerable() // as enumerable used to allow for defaultifempty in join (minor damage to performance)
                join oGroup in _database.OBJECTGROUP on contact.OBJECTGROUPID equals oGroup.OBJECTGROUPID into og from objectGroup in og.DefaultIfEmpty(defaultValue: general)
                where contact.OBJECTCONTACTOWNER.Any(o => o.OBJECTTYPE == type && o.OBJECTID == id)
                // ReSharper disable once PossibleNullReferenceException (it's taken care of by check using .any() )
                group new {contact, objectGroup } by (contact.OBJECTCONTACTPROJECT.Any() ? contact.OBJECTCONTACTPROJECT.FirstOrDefault().OBJECTPROJECT.PROJECTNAME : "General") into pGroup
                orderby pGroup.Key == "General" ? pGroup.Key : "" descending 
                select new ProjectViewModel()
                {
                    ProjectName = pGroup.Key,
                    ProjectId = pGroup.FirstOrDefault() != null ? (pGroup.FirstOrDefault().contact.OBJECTCONTACTPROJECT.FirstOrDefault() != null ? pGroup.FirstOrDefault().contact.OBJECTCONTACTPROJECT.FirstOrDefault().OBJECTPROJECTID : -1) : -1,
                    ContactGroups = (from c in pGroup
                            group c by c.objectGroup into grp
                            let canEdit = ContactsModule.CheckUserRole("EDIT", grp.Key.OBJECTTYPE, grp.Key.GROUPNAME)
                            orderby grp.Key.SORTORDER descending 
                            select new ContactGroupViewModel()
                            {
                                GroupName = grp.Key.GROUPNAME,
                                GroupId = grp.Key.OBJECTGROUPID,
                                CanEdit = canEdit,
                                Contacts = grp.Select(item => new ContactViewModel()
                                {
                                    Id = (int)item.contact.OBJECTCONTACTID,
                                    Name = item.contact.NAME,
                                    Description = item.contact.DESCRIPTION,
                                    Editable = canEdit,
                                    ContactInformation = item.contact.OBJECTCONTACTNUMBER.OrderByDescending(num => num.ISMAININFO).Select(num => new ContactInfoViewmodel()
                                    {
                                        Data = num.NUMBERDATA,
                                        IsMain = num.ISMAININFO > 0,
                                        Type = num.OBJECTCONTACTNUMBERTYPE.NAME
                                    }).ToList()
                                }).ToList()
                            }).ToList()
                }).ToList();

this seems to (on average) take about a 4th of the time the original query needed (still a noticeable time due to the size of database but within acceptable limits)

It's not the database that needs fixing, it's the entities that don't have relations and *the query itself*. If you keep trying to construct your ViewModels in the *LINQ* query you'll get bad performance or runtime errors. — Panagiotis Kanavos, Jul 27 '18 at 11:38
The big bug is still there `_database.OBJECTCONTACT.AsEnumerable() // as enumerable used to allow for defaultifempty in join` no, that doesn't allow anything and the hit is huge. You are loading the entire table into memory. If the *rest* of the query works, EF probably creates some ugly queries that look for specific IDs instead of joining. There's no `OBJECTCONTACT` left to join with. Worst case, it loads all tables involved in memory — Panagiotis Kanavos, Jul 27 '18 at 11:41
Does your database have foreign key relations between the `OBJECT*` tables? In this case you could use the [EF Core Power Tools](https://marketplace.visualstudio.com/items?itemName=ErikEJ.EFCorePowerTools) to reverse engineer the entities from the database — Panagiotis Kanavos, Jul 27 '18 at 11:43
the database has almost no actual relations set-up (the joins are there because the database has extremely few foreign key relations set up), also I do not have access to the project generating the db entities (edmx file) so everything I do has to layer on top of that. This project has showed me the strange stuff that's hiding in old systems with layers upon layers of incremental "small changes" — Dagurdan, Jul 27 '18 at 11:46
Even more reason to *not* mix up the database query with the ViewModel transformation — Panagiotis Kanavos, Jul 27 '18 at 11:48
true. I will probably use this for now as a temporary fix, and start on separating this right away (also start advocating for a migration/db update so that I can get at least some control over the db entities) — Dagurdan, Jul 27 '18 at 11:51
Take a look at [this](https://stackoverflow.com/a/51117648/861716): The strategy for using non-supported methods in LINQ-to-Entities queries is: query the exact amount of data --no more, no less-- then continue in memory. — Gert Arnold, Jul 27 '18 at 12:44

horrible and big LINQ statement optimisation

1 Answers1