Performance: DefaultShapeTableManager

[MatteoPiovanelli-Laser]

We are doing a pass to improve performances on our Orchard applications. In particular, we are focusing on the first load, and appliaction startup.

One relatively slow step in those phases is when the ShapeTableManager has to sort all shapeAlterations. https://github.com/OrchardCMS/Orchard/blob/882fb8eca544c3d87a8634b931df6a4f77b5bd20/src/Orchard/DisplayManagement/Descriptors/DefaultShapeTableManager.cs#L53

By studying the method used for sorting https://github.com/OrchardCMS/Orchard/blob/882fb8eca544c3d87a8634b931df6a4f77b5bd20/src/Orchard/Utility/DependencyOrderingUtility.cs#L18

We figured that all criteria used only involved features, rather than directly the shapeAlterations. So we've been testing a change where we've grouped alterations by feature, and sorted those groups. The algorithm is the same, but it ends up runnig with an order of magnitude fewer items (i.e. up to all features, rather than up to all possible shapes). As such, the sorting step is significantly faster (even 1/10 the time).

We've been testing this on some of our servers for both functionality and performance, and it looks fine. The final ordering of shapes is:

• In absolute, not the same: given the same initial alterationSets, the descriptors collection at the end of the process is not exactly the same; it has of course the same items, but not in the same order.
• Relatively, the same: shape descriptors that are "related" to one another, and need to be in a specific order with respect to each other in the final collection, will be in that order, even though they may be at different indexes that they would be with the sorting method as it is "live".

Because of this, I'm not sure where you'd rather have this PR. AFAIK, there's nothing that depends on the exact absolute indexes of stuff in the shapetables: if you agree, I'd make this PR to 1.10.x; otherwise, I'll make it to dev.

[sebastienros]

There might be some optimizations available here, but it's very risky. The test matrix you are doing is probably very limited (how different are the tenants you load, do they have different shapes, do they have different custom css, ...) and maybe you are introducing bugs when I read that the final order is not exactly the same.

You should at least try even with a few set of tenants that customizations are not broken.

[MatteoPiovanelli-Laser]

The tenants we are testing this on are pretty different from one another. Perhaps next meeting I'm able to join and show you some. We have different features, themes, resources. A bunch of our custom features that further add layers of customization (per-tenant alternates, for example). To test, I've been treating the application as a black box and comparing the source code produced for several dozen pages.

Of course, that doesn't guarantee coverage. I feel confident I haven't missed anything too obvious, but it'll be better to have a further set of eyes on the code. I'll do a PR before next Thursday so we can also go through the code together.