Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

[perf] DefaultActionSelector.HasValidAction allocate significant memory, which can be fixed. #1114

Closed
troydai opened this issue Sep 9, 2014 · 4 comments
Assignees
Milestone

Comments

@troydai
Copy link
Contributor

troydai commented Sep 9, 2014

Following data shows up in MusicStore (CLR) perf profiling:

Name
 Type Enumerator[Microsoft.AspNet.Mvc.RouteDataActionConstraint]
+ CLR <<clr!JIT_New>>
 + LIB <<System.Core.ni!System.Linq.Enumerable.All[System.__Canon](System.Collections.Generic.IEnumerable`1, System.Func`2)>>
  + module Microsoft.AspNet.Mvc.Core <<Microsoft.AspNet.Mvc.Core!Microsoft.AspNet.Mvc.DefaultActionSelector+<>c__DisplayClass20.<HasValidAction>b__21(class Microsoft.AspNet.Mvc.ActionDescriptor)>>
   + LIB <<System.Core.ni!System.Linq.Enumerable.Any[System.__Canon](System.Collections.Generic.IEnumerable`1)>>
    + module Microsoft.AspNet.Mvc.Core <<Microsoft.AspNet.Mvc.Core!MvcRouteHandler.GetVirtualPath>>
     + module Microsoft.AspNet.Routing <<Microsoft.AspNet.Routing!RouteCollection.GetVirtualPath>>
@danroth27 danroth27 changed the title DefaultActionSelector.HasValidAction allocate significant memory can, which can be fixed. DefaultActionSelector.HasValidAction allocate significant memory, which can be fixed. Sep 16, 2014
@danroth27
Copy link
Member

@troydai Can you please more details in the bug on how much memory is actually getting allocated and what scenarios are affected? Thanks!

@danroth27 danroth27 added this to the 6.0.0-rc1 milestone Sep 24, 2014
@danroth27 danroth27 removed this from the 6.0.0-rc1 milestone Sep 24, 2014
@troydai
Copy link
Contributor Author

troydai commented Sep 30, 2014

In a bottom up investigation of Music Store desktop memory consumption, following call took 2.1% of the total memory usage inclusively:

Name                                                                                                                                                                                                Exc %   Exc Exc Ct  Inc %
module Microsoft.AspNet.Mvc.Core <<Microsoft.AspNet.Mvc.Core!Microsoft.AspNet.Mvc.DefaultActionSelector+<>c__DisplayClass20.<HasValidAction>b__21(class Microsoft.AspNet.Mvc.ActionDescriptor)>>      0.0     0      0    2.2

The memory were all spent on

LIB <<System.Core.ni!System.Linq.Enumerable.All[System.__Canon](System.Collections.Generic.IEnumerable`1, System.Func`2)>>

Here's the source codes:

        public virtual bool HasValidAction([NotNull] VirtualPathContext context)
        {
            if (context.ProvidedValues == null)
            {
                // We need the route's values to be able to double check our work.
                return false;
            }

            var actions =
                GetActions().Where(
                    action =>
                        action.RouteConstraints == null ||
                        action.RouteConstraints.All(constraint => constraint.Accept(context.ProvidedValues)));

            return actions.Any();
        }

Should these route constraints be cached, the memory consumption can be reduce to minimal.

@rynowak
Copy link
Member

rynowak commented Oct 1, 2014

we actually already have the cache this would be a straightforward code change to remove some linq from a hot path

@yishaigalatzer
Copy link
Contributor

144a4b5

@yishaigalatzer yishaigalatzer added this to the 6.0.0-beta1 milestone Oct 8, 2014
@yishaigalatzer yishaigalatzer added bug and removed bug labels Oct 8, 2014
@yishaigalatzer yishaigalatzer changed the title DefaultActionSelector.HasValidAction allocate significant memory, which can be fixed. [perf] DefaultActionSelector.HasValidAction allocate significant memory, which can be fixed. Oct 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants