I could be that you're using dynamic. Even though your dynamic objects are of type DynamicNode, they will inherently be slower than instantiating a "real" DynamicNode.
I normally stay clear of dynamic as much as possible.
Instantiate your objects eg.
DynamicNode top = new DynamicNode(Model.Id).AncestorOrSelf(2);
Then get your values with eg top.GetPropertyValue("menuClass")
It's a bit more code but visual studio helps with intellisense.
Some menu items don't need an active state based on theliClass. It's a complicated menu with many options :p. I used Array.IndexOf(Model.Path.Split(','), c.Id.ToString()) >= 0 because that was in one Razor template files when you create a new Razor file in the backend. It was the Navigation example:
@* NAVIGATION BY LEVEL ================================= This snippet makes it easy to do navigation based lists! It'll automatically list all children of a page with a certain level in the hierarchy that's published and visible (it'll filter out any pages with a property named "umbracoNaviHide" that's set to 'true'.
How to Customize for re-use (only applies to Macros, not if you insert this snippet directly in a template): - If you add a Macro Parameter with the alias of "Level" you can use this macro for both level 1 and level 2 navigations - If you add a Macro Parameter with the alias of "ulClass" you can specify different css classes for the <UL/> element
How it works: - The first two lines (var level... and var ulClass) assigns default values if none is specified via Macro Parameters - Then it finds the correct parent based on the level and assigns it to the 'parent' variable. - Then it runs through all the visible children in the foreach loop and outputs a list item - Inside the list item it checks if the page added to the list is a parent of the current page. Then it marks it 'selected'
NOTE: It is safe to remove this comment (anything between @ * * @), the code that generates the list is only the below! *@
@inherits umbraco.MacroEngines.DynamicNodeContext @{ var level = String.IsNullOrEmpty(Parameter.Level) ? 1 : int.Parse(Parameter.Level); var ulClass = String.IsNullOrEmpty(Parameter.UlClass) ? "" : String.Format(" class=\"{0}\"", Parameter.UlClass); var parent = @Model.AncestorOrSelf(level); if (parent != null) { <ul@Html.Raw(ulClass)> @foreach (var item in parent.Children.Where("Visible")) { var selected = Array.IndexOf(Model.Path.Split(','), item.Id.ToString()) >= 0 ? " class=\"selected\"" : ""; <li@Html.Raw(selected)> <a href="@item.Url">@item.Name</a> </li> } </ul> } }
Hmm if I don't use dynamic I don't need to use DynamicNode at all... Could just use the Node factory in that case and I think DynamicNode is created to be dynamic.
When you use @Model.Property this will be translated to DynamicNode.GetPropertyValue("Property") under the hood. This translation can lead to a performance hit.
Well I tried rewriting it to just use DynamicNode which is not dynamic, but the code get's a lot more and I use things like this a lot which also don't work:
top.Children.Where("hideInMenu != true")
So I would really like to keep using dynamic, and I think using that won't make things that much slower. I've used it a lot on other websites and I didn't notice any performance hits.
I'll do that once I find the underlying problem :). Want to have this fixed :p. Anyone know a good tool to test performance? Would like to debug my razor file and see what the real bottleneck might be.
It's a boolean (the true/false datatype). I always use hideInMenu instead of umbracoNaviHide, because we use that for the xml sitemap. Sometimes I want to hide the node in the menu, but still have it in the xml sitemap.
Hmmm... However it relates to the other properties as well... There's things in the trace that looks a bit suspective for me:
RazorDynamicNode got datatype a74ea9c9-8e18-4d2a-8cf6-73c6206c5da6 for menuClass on Home Checking for a RazorDataTypeModel for data type guid a74ea9c9-8e18-4d2a-8cf6-73c6206c5da6... Checking the RazorDataTypeModelTypes static mappings to see if there is a static mapping... There isn't a staticMapping defined so checking the RazorDataTypeModelTypes cache... GUID a74ea9c9-8e18-4d2a-8cf6-73c6206c5da6 does not have a DataTypeModel, falling back to ConvertPropertyValueByDataType
As far as I know from the sources it points to the attempt to convert the property value from a string to some "stronger" datatype basing on the datatype ID. I'm just speculating but perhaps the problem is somewhere there.
@Rodion I think that is a new feature of Umbraco 4.7.1.1. It tries to convert it to a strong type. I already had performance problems with it before (long story, but page load was 120 seconds :p) so perhaps it can be disabled somehow.
@Anthony I really like to use the DyanmicNode as a dynamic. It makes the code clearer and more flexible. Really a last option to drop dynamic.
I think that dynamics aren't necessary a large performance hit. It depends. In the worst case a dynamic property/method is resolved with the reflection what's definitely not good in performance terms. But on the other hand they can be dispatched through DynamicObject.TryXXX(...) that is the case for DynamicNode and should be much faster that dumb reflection.
I was thinking about that, but perhaps it's better to do some profiling first? See what the bottle necks are by placing breakpoints and looking at how long each breakpoint takes. Can I already do that with Visual Studio or do I need an extension for that?
Are you using a plain website? I would like to fix this as well since my latest project has the same messages in the stack trace. I have some time tomorrow, I'll look into it :)
I just had a flashback....I had a similar strange performance issue many months ago.
After removing all other cshtml files and macros from the page, I found that the issue was to do with AncestorOrSelf, and DescendentsOrSelf when used on Model.
The fix was : whenever I needed to do AncestorOrSelf I did :
new DynamicNode(Model.Id).AncestorOrSelf()
This showed a significant improvement.
Also, if you need to do AncestorOrSelf(2) in multiple cshtml files on the page to get the root node, you should make a helper method and cache the result in request cache (ie. HttpContext.Items). It drastically reduces the load time.
After some more emailing with Gareth I got it solved :).
Let me first say this: Dynamic Razor is still good for most situations and it's fast. Only if you want to iterate through a lot of nodes it could cause performance problems.
Here are parts of the email conversation:
First reaction about performance problem: "Well, in short, it's all the property accesses for hideInMenu. The razor datatypes model lookups probably needs some caching love, but that would be an internal fix rather than a fix to your script."
I asked if this will be fixed: "I'm tied up on a project for a few weeks, but I really need to touch base with Niels first. Now u5.0 is out, I know u4 will continue to be used - but I am wondering if my efforts are best spent elsewhere"
Another reaction about the performance problem: "What seems to be calling the slowdown here is the repeated property accesses for hideInMenu that don't seem to be cached (probably a .Where issue, not sure) I was optimising that to use examine but ran into an examine bug in 4.7.0 - I think that's been fixed so in 4.7.2 we will upgrade the examine library (with Niels blessing) It's one of the most used methods so it needs optimisation."
Solution for the Problem: "Yes, load it into a custom object and then use that for your rendering loop - it's less than ideal but seems the best way to solve the performance issue in the interim"
Here is my new code:
Menu.cshtml
@inherits umbraco.MacroEngines.DynamicNodeContext
@using System.Globalization;
@using umbraco.MacroEngines;
@using umbraco;
@using Project.General.BO;
@using Project.WebApplication.Razor;
@{
//Get the top node of the current language.
dynamic top = Helper.GetTop(Model);
//Add the nodes to the menuItems list.
List<Menu> menuItems = new List<Menu>();
foreach (var menuItem in top.DescendantsOrSelf(2))
{
menuItems.Add(new Menu()
{
Id = menuItem.Id,
ParentId = menuItem.Parent.Id,
Path = menuItem.Path,
HideInMenu = (menuItem.GetPropertyValue("hideInMenu") != "0"),
HasActiveState = (menuItem.GetPropertyValue("hasActiveState") == "1"),
MenuClass = menuItem.GetPropertyValue("menuClass"),
MenuTitle = menuItem.GetPropertyValue("menuTitle"),
Url = menuItem.Url
});
}
//Convert the flat list into a nested list and get the first menu item since this is always the top node of the current language.
Menu nestedMenu = Helper.CreateNestedMenu(menuItems)[0];
<ul>
@*Render the home button.*@
@RenderMenu(nestedMenu, nestedMenu.MenuClass, Model.Id == nestedMenu.Id && nestedMenu.HasActiveState, false)
@*Render the other nodes.*@
@foreach (Menu menu in nestedMenu.Children.Where(x => x.HideInMenu == false).ToList())
{
@RenderMenu(menu, menu.MenuClass, Array.IndexOf(Model.Path.Split(','), menu.Id.ToString()) >= 0 && menu.HasActiveState, true)
}
</ul>
}
@*Render the different menu buttons.*@
@helper RenderMenu(Menu menu, string liClass, bool active, bool renderSubmenu)
{
var spanClass = "";
List<Menu> children = menu.Children.Where(x => x.HideInMenu == false).ToList();
bool renderChildren = renderSubmenu && children.Count() > 0;
if (renderChildren)
{
//Change the css class if a submenu will be rendered.
liClass = liClass + "_dropdown";
}
if (active)
{
//Add the active class.
liClass = liClass + " active";
spanClass = " class=\"active\"";
}
<li><a href="@menu.Url" class="@liClass"><[email protected](spanClass)>@menu.MenuTitle</span></a>
@if (renderChildren)
{
<ul>
@foreach (Menu childMenu in children)
{
<li><a href="@childMenu.Url">@childMenu.MenuTitle</a></li>
}
</ul>
}
</li>
}
Menu class:
public class Menu
{
public int Id { get; set; }
public int ParentId { get; set; }
public string Path { get; set; }
public bool HideInMenu { get; set; }
public bool HasActiveState { get; set; }
public string MenuClass { get; set; }
public string MenuTitle { get; set; }
public string Url { get; set; }
public List<Menu> Children = new List<Menu>();
}
Static helper method:
/// <summary>
/// Convert a flat list of menu items in a nested list.
/// </summary>
/// <param name="lines"></param>
/// <returns></returns>
public static List<Menu> CreateNestedMenu(List<Menu> menuItems)
{
var idLookup = new Dictionary<int, Menu>();
var roots = new List<Menu>();
foreach (Menu menu in menuItems)
{
Menu parent;
if (idLookup.TryGetValue(menu.ParentId, out parent))
{
parent.Children.Add(menu);
}
else
{
roots.Add(menu);
}
idLookup.Add(menu.Id, menu);
}
return roots;
}
After this here are the new performance results:
Old menu: 0,547868017647013 New menu: 0,036914941413300
So using this new code made the website 0,5 sec faster on each pageload which is a huge performance boost! Thanks everyone for helping. #h5yr
That's really, really useful. Thanks to Jereon and Gareth for the info. I found similiar Razor performance issues when iterating lots of nodes - I blogged about it briefly (http://jamesdrever.co.uk/blog/?p=246) - I ended up using Examine to sort the issues. But in some cases you really don't want to have to do that. Be really good to see a fix at core level - loading into a custom object is really less than ideal.
Can someone clarify something: Is the performance penalty down to the fact that the "where" clause is operating on a custom property("hideInMenu")? Would it be faster if it was operating on a "built-in" Node property, such as "id"? Or is it nothing to do with that?
Not sure, but I think it's because it's a custom property. The "built-in" properties are already strongly typed so they don't need to be casted. At least that's what I think.
Querying Umbraco data from your Razor files (of views if you use the MVC parts of Umbraco) has been completely refactored. It now uses IPublishedContent for statically typed access. You can also use the dynamic way of getting your data.
@Dimitry - You can append umbDebugShowTrace=true to the URL as a query string parameter. This requires umbracoDebugMode to be set to "true" in the web.config to work.
Another question I have, is with the static helper method, CreateNestedMenu, did you define this method in a separate class file, or did you define it in the Menu.cs class?
I'm looking for a menu navigation script that list not only top pages, but also subpages. The default Umbraco Razor template for main navigation only seems to list top pages but no subpages.
@Jeroen, I implemented your Navigation solution: Navigation.cshtml, Menu.cs and Helper.cs, but apparently the script is complaining about a Helper.getTop() method that is not available in the static Helper class.
Razor menu performance (v4)
Hello,
I've created a menu in Razor, but it seems to be pretty heavy and I'm not sure why. Here is the script:
This is the html output:
I assume this shouldn't take too long, but when I look at the trace this is the output:
It takes almost half a second to just load the menu so something must wrong. Any idea how to increase performance?
Jeroen
This is how I call the Razor script in my template:
Would it make any difference to make a macro in the backoffice and use that?
Jeroen
Why do you use Array.IndexOf(Model.Path.Split(','), c.Id.ToString()) >= 0 && c.hasActiveState to check if the menu needs to be active ?
Can't you use c.IsAncestor(Model) ?
I could be that you're using dynamic. Even though your dynamic objects are of type DynamicNode, they will inherently be slower than instantiating a "real" DynamicNode.
I normally stay clear of dynamic as much as possible.
Instantiate your objects eg.
Then get your values with eg top.GetPropertyValue("menuClass")
It's a bit more code but visual studio helps with intellisense.
Some menu items don't need an active state based on the liClass. It's a complicated menu with many options :p. I used Array.IndexOf(Model.Path.Split(','), c.Id.ToString()) >= 0 because that was in one Razor template files when you create a new Razor file in the backend. It was the Navigation example:
Jeroen
Hmm if I don't use dynamic I don't need to use DynamicNode at all... Could just use the Node factory in that case and I think DynamicNode is created to be dynamic.
Jeroen
Jeroen
That's not entirely true.
When you use @Model.Property this will be translated to DynamicNode.GetPropertyValue("Property") under the hood. This translation can lead to a performance hit.
I changed this:
Into this:
But performance is the same.
Jeroen
Well I tried rewriting it to just use DynamicNode which is not dynamic, but the code get's a lot more and I use things like this a lot which also don't work:
So I would really like to keep using dynamic, and I think using that won't make things that much slower. I've used it a lot on other websites and I didn't notice any performance hits.
Jeroen
In your RenderMenu function you have this twice : c.Children
Maybe you should store that in a variable
Made it a variable, but no difference. All these things only have a really small impact on performance. I must be missing something else.
Jeroen
"Would it make any difference to make a macro in the backoffice and use that?"
It probably wouldn't solve the underlying problem, but at least you would be able to add caching.
I'll do that once I find the underlying problem :). Want to have this fixed :p. Anyone know a good tool to test performance? Would like to debug my razor file and see what the real bottleneck might be.
Jeroen
Out of interest, what database are you using? SQL Server or embedded?
I'm using SQL Server, but does that have anything to do with this Razor file? It only runs against the umbraco.config xml file.
Jeroen
Hi, Jeroen. What is the datatype of the "hideInMenu" property? (I suspect that it's some custom one)
It's a boolean (the true/false datatype). I always use hideInMenu instead of umbracoNaviHide, because we use that for the xml sitemap. Sometimes I want to hide the node in the menu, but still have it in the xml sitemap.
Jeroen
Oh, no. I'm wrong. It's the "menuTitle" property that's of my interest not the "hideInMenu".
That's just a Textstring like other fields. Could use the node name there, but we prefer to keep those separate :).
Jeroen
Hmmm... However it relates to the other properties as well... There's things in the trace that looks a bit suspective for me:
As far as I know from the sources it points to the attempt to convert the property value from a string to some "stronger" datatype basing on the datatype ID. I'm just speculating but perhaps the problem is somewhere there.
Rodion the whole RazorDynamicNode thing is because of the use of dynamic. There will always be a performance hit when using dynamic.
Jeroen I would just use NodeFactory or DynamicNode. I use extension methods to keep my code concise.
@Rodion I think that is a new feature of Umbraco 4.7.1.1. It tries to convert it to a strong type. I already had performance problems with it before (long story, but page load was 120 seconds :p) so perhaps it can be disabled somehow.
@Anthony I really like to use the DyanmicNode as a dynamic. It makes the code clearer and more flexible. Really a last option to drop dynamic.
Jeroen
I think that dynamics aren't necessary a large performance hit. It depends. In the worst case a dynamic property/method is resolved with the reflection what's definitely not good in performance terms. But on the other hand they can be dispatched through DynamicObject.TryXXX(...) that is the case for DynamicNode and should be much faster that dumb reflection.
I'm very curious what this performance issue is.
Jeroen, perhaps you just need to do some old fashion brute force investigation.
Remove everything until you have no performance issue, then add lines/sections until you see an issue.
I was thinking about that, but perhaps it's better to do some profiling first? See what the bottle necks are by placing breakpoints and looking at how long each breakpoint takes. Can I already do that with Visual Studio or do I need an extension for that?
Jeroen
I have yet another stupid idea. What if to try to set either RenderEvent="Init" or EnableViewState="false" on that <umbraco:macro> control?
@Jeroen:
Are you using a plain website? I would like to fix this as well since my latest project has the same messages in the stack trace.
I have some time tomorrow, I'll look into it :)
No sure what you mean by plain website, but it's the same as the last website which I sent you a while back :).
Jeroen
Same database and code? :)
Great, I'll test it with that website :)
No different website and database, but the same structure :). I could send it again if you want.
Jeroen
If you could send that website to me I can debug it directly using my own build of Umbraco. I'll try to find out what the problem is tomorrow :)
Have you tried to remove the .Where("hideInMenu != true") just to see if that's what causing the perf hit?
Is it a True/False DataType?
Do you have an IRazorDataTypeModel defined for the True/False DataType?
I tried removing that and it didn't make much difference. I don't have done anything special for the True/False datatype.
I sent the code to Floris and he's looking at it. Also Gareth might have a look at it.
Jeroen
I just had a flashback....I had a similar strange performance issue many months ago.
After removing all other cshtml files and macros from the page, I found that the issue was to do with AncestorOrSelf, and DescendentsOrSelf when used on Model.
The fix was : whenever I needed to do AncestorOrSelf I did :
new DynamicNode(Model.Id).AncestorOrSelf()
This showed a significant improvement.
Also, if you need to do AncestorOrSelf(2) in multiple cshtml files on the page to get the root node, you should make a helper method and cache the result in request cache (ie. HttpContext.Items). It drastically reduces the load time.
I've already had a talk with Gareth and I know the problem. It's should be related to .Where("hideInMenu != true"). Will post more info soon.
Jeroen
After some more emailing with Gareth I got it solved :).
Let me first say this: Dynamic Razor is still good for most situations and it's fast. Only if you want to iterate through a lot of nodes it could cause performance problems.
Here are parts of the email conversation:
First reaction about performance problem:
"Well, in short, it's all the property accesses for hideInMenu. The razor datatypes model lookups probably needs some caching love, but that would be an internal fix rather than a fix to your script."
I asked if this will be fixed:
"I'm tied up on a project for a few weeks, but I really need to touch base with Niels first. Now u5.0 is out, I know u4 will continue to be used - but I am wondering if my efforts are best spent elsewhere"
Another reaction about the performance problem:
"What seems to be calling the slowdown here is the repeated property accesses for hideInMenu that don't seem to be cached (probably a .Where issue, not sure) I was optimising that to use examine but ran into an examine bug in 4.7.0 - I think that's been fixed so in 4.7.2 we will upgrade the examine library (with Niels blessing) It's one of the most used methods so it needs optimisation."
Solution for the Problem:
"Yes, load it into a custom object and then use that for your rendering loop - it's less than ideal but seems the best way to solve the performance issue in the interim"
Here is my new code:
Menu.cshtml
Menu class:
Static helper method:
After this here are the new performance results:
Old menu: 0,547868017647013
New menu: 0,036914941413300
So using this new code made the website 0,5 sec faster on each pageload which is a huge performance boost! Thanks everyone for helping. #h5yr
Jeroen
That's really, really useful. Thanks to Jereon and Gareth for the info. I found similiar Razor performance issues when iterating lots of nodes - I blogged about it briefly (http://jamesdrever.co.uk/blog/?p=246) - I ended up using Examine to sort the issues. But in some cases you really don't want to have to do that. Be really good to see a fix at core level - loading into a custom object is really less than ideal.
I still think this performance issue can be fixed in the core. When I find the time I'll give it try :)
Can someone clarify something: Is the performance penalty down to the fact that the "where" clause is operating on a custom property("hideInMenu")? Would it be faster if it was operating on a "built-in" Node property, such as "id"? Or is it nothing to do with that?
Not sure, but I think it's because it's a custom property. The "built-in" properties are already strongly typed so they don't need to be casted. At least that's what I think.
Jeroen
Has anyone logged a bug on Codeplex for this? Think it should definitely be looked at.
I don't it has been logged on Codeplex yet.
Jeroen
Pls vote this up :)
http://umbraco.codeplex.com/workitem/30745
Does anyone know if this has been fixed?
@Michael:
Querying Umbraco data from your Razor files (of views if you use the MVC parts of Umbraco) has been completely refactored. It now uses IPublishedContent for statically typed access. You can also use the dynamic way of getting your data.
You can find all the documentation about this new way of getting data here: http://our.umbraco.org/documentation/reference/mvc/querying/
How is it possible to get such detailed trace for macro rendering with razor?
@Dimitry - You can append umbDebugShowTrace=true to the URL as a query string parameter. This requires umbracoDebugMode to be set to "true" in the web.config to work.
@Dan Thanks, i know about this function. It is not giving such a detailed trace as Jeroen was posting here. So the question is still in the air
Hi Jeroen,
Thanks for sharing your solution.
I would like to implement this in my website.
Just a question. I guess this namespace belong to your particular webproject:
Another question I have, is with the static helper method, CreateNestedMenu, did you define this method in a separate class file, or did you define it in the Menu.cs class?
Thanks for your advice,
Anthony
Yes those are namespaces from my web project. The static helper is a separate class file. It's not in the Menu.cs class.
If you're using MVC in 4.10 or higher you don't have this problem anymore.
Jeroen
@Jeroen, oh I see. I'm using Umbraco v4.11.4
I'm looking for a menu navigation script that list not only top pages, but also subpages. The default Umbraco Razor template for main navigation only seems to list top pages but no subpages.
greetings,
Anthony
@Jeroen, I implemented your Navigation solution: Navigation.cshtml, Menu.cs and Helper.cs, but apparently the script is complaining about a Helper.getTop() method that is not available in the static Helper class.
Could you post the Helper.getTop() method?
Thanks,
Anthony
Hi Jeroen,
No need for the getTop() Helper method. I extended the default Umbraco Navigation script, it was easier than I thought:
@inherits umbraco.MacroEngines.DynamicNodeContext
@{
var level = String.IsNullOrEmpty(Parameter.Level) ? 1 : int.Parse(Parameter.Level);
var ulClass = String.IsNullOrEmpty(Parameter.UlClass) ? "" : String.Format(" class=\"{0}\"", Parameter.UlClass);
var parent = @Model.AncestorOrSelf(level);
if (parent != null) {
<[email protected](ulClass)>
@foreach (var item in parent.Children.Where("Visible")) {
var active = Array.IndexOf(Model.Path.Split(','), item.Id.ToString()) >= 0 ? " class=\"active\"" : "";
<[email protected](active)>
<a href="@item.Url">@item.Name</a>
@if(item.Children.Items.Count != 0){
var subpages = item.Children;
<ul>
@foreach (var subpage in subpages)
{
<li><a href="@subpage.Url">@subpage.Name</a></li>
}
</ul>
}
</li>
}
</ul>
}
}
Hope this might be of help to someone else
greetings,
Anthony
is working on a reply...