1
\$\begingroup\$

I'm creating a filter with Linq in an ASP.NET MVC application. I was hoping I could get some tips on performance enhancements, aside from any possible performance enhancements.

I have a few questions:

  • Should I ToList before my extension methods that use .Select?
  • Would adding AsNoTracking help performance? If so, if my GetAllIncluding method returns IEnumerable, where should I add AsQueriable().AsNoTracking()?
  • The Entities SubProject and Activity are self referencing Entities I'm trying to eager load them as all the lazy loading looping is extremely slow. Am I doing this in the correct manner?

I know this is using Repo pattern and UOW, but that's just the way this app was built so I would like to build this around that.

Here is the DTO I'm sending to the client:

public class GanttDto
{
    public IEnumerable<ProductLineDto> ProductLines { get; set; }
    public IEnumerable<ProjectDto> Projects { get; set; }
    public IEnumerable<SubProjectDto> Subprojects { get; set; }
    public IEnumerable<ActivityDto> Activities { get; set; }
    public IEnumerable<ProjectTypeDto> ProjectTypes { get; set; }
}

Here are my filter methods

   public GanttDto Filter(GanttFilterDto filterCriteria)
    {

        FilterCriteria = filterCriteria;

        var projects = FilterProjects().ToList();

        var pIds = projects.Select(x => x.ProjectID);

        var data = new GanttDto {
            ProductLines = FilterProductLines(),
            Projects = projects.ToProjectDtoEnumerable(),
            Subprojects = FilterSubProjects(pIds),
            Activities = GetActivities(pIds),
            ProjectTypes = UnitOfWork.ProjectTypeRepository.GetAll().OrderBy(z => z.SortOrder).ToList().ToProjectTypeDtoEnumerable()
        };

        return data;  
    }

Here is the GetAllIncluding method I'm using for each method below:

    public IEnumerable<TEntity> GetAllIncluding(params Expression<Func<TEntity, object>>[] includesProperties)
    {
        return includesProperties.Aggregate<Expression<Func<TEntity, object>>,
        IQueryable<TEntity>>(_dbSet, (current, includeProperty)
     => current.Include(includeProperty));
    }

Filter projects

   private IEnumerable<Project> FilterProjects()
    {
        var productLineIds = FilterCriteria.SelectedProjects.Where(x => x.ProductLineId != null).Select(x => x.ProductLineId).ToList();
        var projectTypeIds = FilterCriteria.SelectedProjects.Where(x => x.ProjectTypeId != null).Select(x => x.ProjectTypeId).ToList();
        var projectIds = FilterCriteria.SelectedProjects.Where(x => x.ProjectId != null).Select(x => x.ProjectId).ToList();


        return UnitOfWork.ProjectRepository.GetAllIncluding(x => x.ProjectPersons,
                                                                    x => x.ProjectPersons.Select(y => y.ProjectPersonRoles))

            .Where(prj => ((productLineIds.Count == 0 || productLineIds.Any(id => id == prj.ProductLineID)) //filter by ProductLine
                          && (projectTypeIds.Count == 0 || projectTypeIds.Any(id => id == prj.ProjectTypeID)) // ProjectType
                           && (projectIds.Count == 0 || projectIds.Any(id => id == prj.ProjectID)) // Project
                           && (FilterCriteria.StatusId.Contains(prj.ProjectStatusTypeID)) //project status

                           )).ToList();
    }

To Project Dto

    public static IEnumerable<ProjectDto> ToProjectDtoEnumerable(this IEnumerable<Project> projects)
    {
        return projects.Select(x => new ProjectDto
        {
            ProjectId = x.ProjectID,
            ProjectName = x.Name,
            StartDate = x.StartDate,
            //Children = x.SubProjects.Where(y => y.ParentSubProjectID == null).ToSubProjectDtoEnumerable(),
            Organization = x.Organization.ToOrganizationDto(),
            ProjectStatus = x.ProjectStatusType.Description,
            ProjectTypeRowId = x.ProjectType.RowID,
            ProjectTypeDescription = x.ProjectType.Description,
            SortOrder = x.SortOrder,
            ProjectPersons = x.ProjectPersons.ToProjectPersonDtoEnuemrable(),
            ProjectStatusId = x.ProjectStatusTypeID,
            ProductLineId = x.ProductLineID,
            ProjectTypeId = x.ProjectTypeID,
            OrganizationId = x.OrganizationID,
            ProductLineName = x.ProductLine.Description,
            IsShotgun = x.IsShotgun,
            ShotgunLink = x.ShotgunLink,
            Description = x.Description,
            Flow = x.Flow,
            ModelType = x.ModelType,
            ModelYear = x.ModelYear,
            BudgetCode = x.BudgetCode,
            ProjectCode = x.ProjectCode,
            ProjectLeaderDescription = x.ProjectLeaderDescription
        });
    }

Filter ProductLines

    private IEnumerable<ProductLineDto> FilterProductLines() {

        var productLineIds = FilterCriteria.SelectedProjects.Where(x => x.ProductLineId != null).Select(x => x.ProductLineId).ToList();

        return UnitOfWork.ProductLineRepository.Where(x => productLineIds.Count == 0 || productLineIds.Contains(x.ProductLineID)).OrderBy(z => z.SortOrder).ToList().ToProductLineDtoEnumerable();
    }

To ProductLine Dto

    public static IEnumerable<ProductLineDto> ToProductLineDtoEnumerable(this IEnumerable<ProductLine> productLines)
    {
        return productLines.Select(x => new ProductLineDto
        {
            Name = x.Description,
            ProductLineId = x.ProductLineID,
            SortOrder = x.SortOrder
        });
    }

Filter SubProjects

    private IEnumerable<SubProjectDto> FilterSubProjects(IEnumerable<Guid> projectIds)
    {

        var sp = UnitOfWork.SubProjectRepository.GetAllIncluding(x => x.SubProject1,
            x => x.SubProjectPersons,
            x => x.SubProjectPersons.Select(y => y.SubProjectPersonRoles),
            x => x.SubProjectTeams)

            .Where(y => y.ParentSubProjectID == null && projectIds.Contains(y.ProjectID)).OrderBy(z => z.SortOrder).ToList();

        return sp.ToSubProjectDtoEnumerable();
    }

To SubProjectDto

public static IEnumerable<SubProjectDto> ToSubProjectDtoEnumerable(this IEnumerable<SubProject> projects)
        {
            return projects.Select(x => new SubProjectDto
            {
                ProjectId = x.ProjectID,
                SubProjectId = x.SubProjectID,
                ProjectName = x.Name,
                Children = x.SubProject1.Where(y => y.ParentSubProjectID == x.SubProjectID).ToSubProjectDtoEnumerable(),
                ParentId = x.ParentSubProjectID,
                SubProjectPersons = x.SubProjectPersons.ToSubProjectPersonDtoEnuemrable(),
                SubProjectTypeId = x.SubProjectTypeID,
                OrganizationId = x.OrganizationID,
                SortOrder = x.SortOrder,
                IsShotgun = x.IsShotgun,
                ExpandToCustomLevel = x.expandToCustomLevel,
                Teams = x.SubProjectTeams?.ToSubProjectTeamDtoEnumerable()
            });
        }

Filter Activities

private IEnumerable<ActivityDto> GetActivities(IEnumerable<Guid> projectIds) {

            if (FilterCriteria.ActivityTypeId == null)
                FilterCriteria.ActivityTypeId = new List<Guid>();

            if (FilterCriteria.MileStoneTypeId == null)
                FilterCriteria.MileStoneTypeId = new List<Guid>();

            var acts = UnitOfWork.ActivityRepository.GetAllIncluding(
                x => x.ActivityTypeCustomForms,
                x => x.Activity1,
                x => x.ActivityPersons,
                x => x.ActivityMachines,
                x => x.ActivityType
                ).Where(act =>

                (FilterCriteria.MileStoneTypeId.Contains(act.ActivityTypeID))
                ||
                (act.ParentActivityID == null && projectIds.Contains(act.ProjectID.Value)) //by project

                &&// start date  - if start date is between start and end get them too
                (FilterCriteria.StartDateFrom == null || (((act.EndDate >= FilterCriteria.StartDateFrom) && (act.StartDate <= FilterCriteria.StartDateFrom)) ||
                                                                                                                                     ((act.StartDate >= FilterCriteria.StartDateFrom && act.EndDate <= FilterCriteria.StartDateFrom)) ||
                                                                                                                                   (act.StartDate >= FilterCriteria.StartDateFrom)))
                &&// End date  - if End date is between start and end get them too
                (FilterCriteria.EndDateFrom == null || (((act.EndDate >= FilterCriteria.EndDateFrom) && (act.StartDate <= FilterCriteria.EndDateFrom)) ||
                                                                                                                                 ((act.StartDate >= FilterCriteria.EndDateFrom && act.EndDate <= FilterCriteria.EndDateFrom)) ||
                                                                                                                                 (act.EndDate <= FilterCriteria.EndDateFrom)))
                && //activity type
                (FilterCriteria.ActivityTypeId.Contains(act.ActivityTypeID))

                &&//resource type 
                (FilterCriteria.ResourceTypeIds == null || act.ActivityPersons.Any(ap => FilterCriteria.ResourceTypeIds.Contains(ap.ResourceTypeID.Value)))
                &&//PersonID 
                (FilterCriteria.PersonId == null || act.ActivityPersons.Any(ap => (ap.PersonID != null && FilterCriteria.PersonId.Contains(ap.PersonID.Value))))

                ).OrderBy(z => z.SortOrder).ToList().ToActivityDtoEnumerable();

            return acts;
        }

To Activity Dto

public static IEnumerable<ActivityDto> ToActivityDtoEnumerable(this IEnumerable<Activity> activities)
        {


            return activities.Select(x => new ActivityDto
            {
                ActivityId = x.ActivityID,
                ActivityName = x.Name,
                Description = x.SpecialDescription,
                ActivityType = x.ActivityType.ToActivityTypeDto(),
                ActivityDateRange = new DateRange(x.StartDate, x.EndDate),
                StartDate = x.StartDate,
                EndDate = x.EndDate,
                Duration = x.Duration,
                PctComplete = x.PctComplete,
                Name = x.Name,
                ActivityPersons = x.ActivityPersons.ToActivityPersonDtoEnumerable(),
                ActivityMachines = x.ActivityMachines.ToActivityMachineDtoEnumerable(),
                Organization = x.Organization.ToOrganizationDto(),
                OrganizationId = x.OrganizationID,
                SortOrder = x.SortOrder,
                ProjectName = x.Project == null ? "Not Found" : x.Project.Name,
                PlannedStart = x.EarliestStartDate,
                PlannedEnd = x.EarliestEndDate,
                LaserProcessId = x.LaserProcessID,
                LaserThicknessId = x.LaserThicknessID,
                MaterialId = x.MaterialID,
                MillingMinimumRadiusId = x.MillingMinimumRadiusID,
                MillingScallopHeightId = x.MillingScallopHeightID,
                ModelCategoryId = x.ModelCategoryID,
                ModelTypeId = x.ModelTypeID,
                RpfdmNozzleId = x.RPFDMNozzleID,
                RpfillTypeId = x.RPFillTypeID,
                RpsparseId = x.RPSparseID,
                VarianceId = x.VarianceID,
                QuantityId = x.QuantityID,
                SpecNotes = x.SpecNotes,
                Children = x.Activity1.ToActivityDtoEnumerable(),
                CustomForm = x.ActivityTypeCustomForms.Any() ? x.ActivityTypeCustomForms.ToActivityCustomFormDto() : new List<ActivityTypeCustomFormDto>(),
                PartsDetailsDescription = x.PartsDetailsDescription,
            });
        }
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Simple optimization is not using IEnumerable.Contains() but using HashSet or Dictionary will reduce number of loops. I would change the 3 vars by HashSet since they are all list of native values (Ids) \$\endgroup\$ Commented Sep 30, 2019 at 14:33

1 Answer 1

4
\$\begingroup\$

It might be boring to hear it again, but I have to ask :)

  1. Why do you think your application need performance optimisations?
  2. Is your bottle neck in these parts of code which you provided?
  3. Maybe problem is with database/external service/network?
  4. Have you measured processing and communication time? Stopwatch
  5. Do you have some kind of monitoring of application, that shows exactly that its performance is degrading? Grafana
  6. Have you tried benchmarking? BenchmarkDotNet
  7. How about full GC? Pro .NET memory management
  8. Have you tried profiling? PerfView

If you're not sure if it's a problem, then don't do optimisations. As Donald Knuth said

Programmers waste enormous amounts of time thinking about, or worrying about, the speed of noncritical parts of their programs, and these attempts at efficiency actually have a strong negative impact when debugging and maintenance are considered. We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil. Yet we should not pass up our opportunities in that critical 3%.

Back to the code, it's my personal opinion, but I don't like LINQ, because of implicit allocations which lead to pressure on GC (you've made few of them). Also I don't get it, why you're invoking ToList() method? You operate on IEnumerables everywhere. If you invoke ToList() then you're allocating list and killing main feature of LINQ - laziness. Condition you wrote is really hard to read, I would work on that to simplify it somehow. I know it's not related with performance, but it's still code review :).

\$\endgroup\$
2
  • 1
    \$\begingroup\$ You write that only profiled/benchmarked code should be optimized but at the same time treat linq as being always bad. This is simply not true. I have a lot o libraries built virtually entirely with linq and they are super fast. I cannot imagine doing it any other way which would mean a lot of work. The few edge cases where you need the last tick of prefomance don't prove linq bad. On the contrary, linq is best for the majority of tasks. That's why you get +1 for the first part and - 1 for the second one. The sum is 0. \$\endgroup\$ Commented Sep 19, 2019 at 10:24
  • 1
    \$\begingroup\$ I totally agree that LINQ is best for the majority of tasks. I didn't say that it's bad, I said that I don't like it, it's not the same :) as in attached blog post it can lead to some implicit allocations that can put more pressure on GC. In OP code you can find few of them, but as I wrote in the first part of the answer - measure, measure, measure. I'm sorry if my answer wasn't clear enough. I didn't want to discredit anything. Also, thank you for your comment. It was very constructive and I will have in my all your points in mind. \$\endgroup\$ Commented Sep 19, 2019 at 11:56

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.