GetAreaName Extension method thread contention or race condition

[marafiq]

In MVC 5.3 I am observing that multiple threads get blocked win the GetArea extension method when rendering child action. How should this be fixed or read as seen in attached traces?

Is it read contention or possible race condition?

[halter73]

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project (ideally a GitHub repo) that illustrates the problem.

The profile screenshot shows DashboardSection.cshtml calling LINQ's ToArray methods, and inside the ToArray callback calling HtmlHelper.Action(string, string, ...?). It's not clear what the last argument is because it's cut off, but I'm assuming it's route values that need to be merged. It's not surprising this is taking a lot of the profile when executed in a tight loop by ToArray.

It's unclear what any of the screenshotted code has to do with the profile, and why you think GetArea (or is it GetAreaName?) is blocked. I see nothing to suggest that from the profile. The profile only shows time being spent in Dictionary<,> which is presumably the RouteValueDictionary. I'm not sure why we'd jump to the conclusion there's a race or read contention rather than that being a very hot path that's getting executed a lot.

[marafiq]

Thank you @halter73 for the response. I meant to ask a question based on what I was seeing in the trace. I did not mean to conclude that there is race condition. It would be hard for me to reproduce the hot path in minimal repository. I will try to paint the overall context to clarify my questions.

Its MVC 5.2.7 application that has 4000 routes in MVC Route Collection and then additional 4000 routes for Link Generation Routes which is causing lot of allocation that leads to GC thus eventual blocking. The allocations happens both on finding the route or rendering an action or generating the URL (When used without named route) for different routes.

We do have plenty of child actions but child action rendering extension methods does not accept named route as the action helper always pass the route name as null when finding virtual path for the area. With thousands of routes split across many ~ 15 areas it leads to excessive allocations in some cases up to 30 MB just for finding one route with default attribute based MVC routing. Since AJAX extension methods also do not accept named route when rendering actions so similar allocations happens there.

If action rendering extension methods (Including AJAX Extension Methods) support named routes, our problem can be mitigated to a great degree because then GetVirtualPathForArea will return early without calling FilterRouteCollectionByArea, I wonder if such change would be possible or what kind of data you would expect me to provide to consider such change a possibility? So this Html.Action("ActionName", "ControllerName", new { Id = Model.Id })) will be Html.Action("ActionName", "ControllerName","RouteName", new { Id = Model.Id }))

2nd Workaround could be: if MVC allowed to hook up, how to filter the route collection by area name as we can have already filtered collections for each area and would save lot of allocations on each pass. Currently I do not see a workaround to override this behavior. Is there a workaround since route collection mostly do not change dynamically at least not for us?

  private static RouteCollection FilterRouteCollectionByArea(RouteCollection routes, string areaName, out bool usingAreas)
  {
      if (areaName == null)
      {
          areaName = String.Empty;
      }

      usingAreas = false;

      // Ensure that we continue using the same settings as the previous route collection
      // if we are using areas and the route collection is exchanged
      RouteCollection filteredRoutes = new RouteCollection()
      {
          AppendTrailingSlash = routes.AppendTrailingSlash,
          LowercaseUrls = routes.LowercaseUrls,
          RouteExistingFiles = routes.RouteExistingFiles
      };

      using (routes.GetReadLock())
      {
          foreach (RouteBase route in routes)
          {
              string thisAreaName = AreaHelpers.GetAreaName(route) ?? String.Empty;
              usingAreas |= (thisAreaName.Length > 0);
              if (String.Equals(thisAreaName, areaName, StringComparison.OrdinalIgnoreCase))
              {
                  filteredRoutes.Add(route);
              }
          }
      }

      // if areas are not in use, the filtered route collection might be incorrect
      return (usingAreas) ? filteredRoutes : routes;
  }

Other Mitigations: I have looked into optimizing the routes resolution and was able to reduce a good number of allocations by using the trick to compare the first path of the route before handing it over to MVC matching. But even after that fix, there are large number of allocations primarily due to no support for rendering named routes while rendering action or child action.

[marafiq]

Roughly I proposing this change to Optimize Filter when app has lot of routes .

I have not provided a reproduceable repo, but in our traces it shows as one of major contributor, like above an example below - it took about 42 seconds - Thread Time Stacks Window while filtering the route collection by area (It includes time spent in GC trying to allocate more space). What would be sufficient to land a change mentioned above if it makes sense? Also having said that, I am can try preparing benchmark with lots of routes to see the impact if a change like this can land.

[mkArtakMSFT]

Thanks for additional information, @marafiq. Unfortunately, there is not yet enough evidence that there is a race condition in the framework. If you would have a memory dump that pinpoints the problem, we will happily take a look at it. Otherwise, this still feels to be due to some issues in your own codebase. We recommend in these situations to try to get help from the community asking these questions in the StackOverflow forum.