OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Orchard ContentItem.Has performance #7992

Open rfcdejong opened 6 years ago

rfcdejong commented 6 years ago

After drilling down more on performance I found another hit, just small but every bit helps

One request on a part of our site resulting into 108,250 hitcount of the Has method only 0.170 sec with the profiler attached.

Reading Jon Skeet his answer it is better to use a static Type typeof https://stackoverflow.com/questions/353342/performance-of-object-gettype/353435#353435

ContentMangementContentItem.cs line 32

return partType == typeof(ContentItem) || _parts.Any(partType.IsInstanceOfType);

replace by a static private static readonly Type ContentItemType = typeof(ContentItem);

and then the line return partType == ContentItemType || _parts.Any(partType.IsInstanceOfType);

PS: I know I should create a PR, will try to do that later..

rfcdejong commented 6 years ago

orchard_contentitem_has

sebastienros commented 6 years ago

I would assume we don't call Has<ContentItem> that often, and the second part of the condition is the one that is costly.

ViRuSTriNiTy commented 6 years ago

Hi there, as you are profiling a lot, would you mind to "profile" my issue? It should show a huge performance gain: https://github.com/OrchardCMS/Orchard/issues/7073

rfcdejong commented 6 years ago

Hi @sebastienros you are right about the second part being costly, but as you can see in the screenshot above it is called that often in just 1 action that takes 4 seconds (with profiler attached). The change from above did almost do nothing, the profiler showed a very tiny improvement, but would only make sense on a huge request rate by very high concurrent usage.

@ViRuSTriNiTy I stopped profiling for now as most performance issues are fixed by manual caching contentitems for lookups with singleton lifetime.

sebastienros commented 6 years ago

I am ok with doing any change, though I am questioning that using static member is faster than inlining the typeof call, did you measure that independently with BenchmarksDotNet for instance?