Performance overhaul

[thecodejunkie]

Even though we have great performance, there is always more that can be done. This issue will only track what is needed and what has been done. Each change should be logged as a different issue and tracked in here. If you have suggestions on things that should be included in this list, please post a comment so it can be added to the list.

[damianh]

So where are the performance concerns?

[thecodejunkie]

You tell me :) Performance is something that can always been improved. For instance the use of Activator.CreateInstance in model binding could perhaps be improved, as as caching reflection members and perhaps json/xml (de)serialization? The idea is to build up a task list of things that we want to take a look at, such as we did with the #1265 epic

[ontehfritz]

This is may not be entirely Nancy specific, but I think it is in valid in the realm of testing performance as Nancy is cross platform and performance should be checked across platforms. It would be "ideal" for Nancy to perform equally or almost equally across platforms. I use Nancy on Mac and Linux mostly and I have noticed some quirks with fastcgi-mono-server-*/nginx it would be great to have Nancy benchmarks cross platform/hosts.

Here is a series of articles that go through performance on ubuntu using different mods and webservers using mono http://forcedtoadmin.blogspot.nl/2013/11/servicestack-performance-in-mono-p1.html Links to the next articles are at the bottom of each article. This is not Nancy specific, but will affect Nancy applications, especially when using asp.net host. (Also can't validate the truth of the benchmarks)

According to the articles there is issues such as memory leaks etc ... from the mono project that contribute to instabilities on *nix systems. It would be great to put pressure on the projects to up the quality and make it stable to IIS ~performance.

I know from experience fastcgi-mono-server has to be reset periodically when there is high traffic ... it would be great to know the short comings on each platform, so they can be dealt with. Nancy is great! I believe it can really spear head cross platform .NET web development which I believe is really important!

[Jevonius]

Short version: A task of 'improve performance' is significantly easier if there is an appropriate set of benchmarks available that enable identification of problem code and can be used to validate improvements.

Long version: This caught my eye having spent some time spelunking through the Nancy code base tracking down some issues with the Razor view engine. I'm not clear what the timescales are for this, presumably you intend for it to be an on-going effort? There's no description for the Future milestone (or any of them for that matter, that I can find - new to GH!) so I'm a little unclear. I'm also not sure how specific you want to be, and what level of change you want to make/consider, e.g. optimisation at the method level, or adjustments requiring significant re-engineering and breaking changes.

There is an obvious risk with a task as general as 'improve performance' of there being a huge amount of premature optimisation, particularly where there is no easy benchmarking/profiling available to clearly demonstrate that a particular change has had a significant/worthwhile/actual-rather-than-perceived improvement. Without a repeatable (and appropriately granular) set of benchmarks, that can be run against old and new code, there is no way I can say that a specific improvement (or otherwise) is down to [my] code changes, gremlins, or planetary alignment!

One example - there is a method in Nancy.Responses.Negotiation.DefaultResponseNegotiator called TryCastResultToResponse that currently looks like:

try
{
    response = (Response) routeResult;
    return true;
}
catch
{
    response = null;
    return false;
}

Given the expense of throwing exceptions, particularly for conditions that it should be possible to test for, my initial thought was to rewrite this as:

response = routeResult as Response;
return routeResult != null && response != null;

This avoids throwing an [expensive] exception, but broke tests - it doesn't work because of the implicit operators defined on Response. I therefore moved the functionality to a TryCast method on Response:

if (
    routeResult is Response
    || routeResult is int
    || routeResult is HttpStatusCode
    || routeResult is string
    || routeResult is Action<Stream>
    || routeResult is DynamicDictionaryValue
    )
{
    response = routeResult;
    return true;
}
else
{
    response = null;
    return false;
}

At this point, I'll pull back out of the rabbit hole. This method works (and the existing tests pass - no pull request as there are no unit tests around the new method [yet]), but I have no idea if this is a worthwhile improvement. An exception is no longer thrown, but has throughput actually improved? This method stuck out to me when I was tracing through a single request trying to understand Nancy, but if it's only called once per request it's probably not even worth worrying about.

During the same tracing I ended up going through DefaultRouteInvoker.Invoke countless times which was going to be another suggestion (i.e. what's the pipeline for a request, is it doing similar things over and over?) but I've not been able to replicate the situation today so I'm putting it down to "I must have done something silly". Again, a clear benchmark would help.

I was also struck by the heavy usage of dynamic (as far as I can tell - again, I'm new to debugging Nancy!), including in the method above. Having previously profiled a project that made use of Simple.Data (which uses dynamic heavily) to try and identify where the slowdowns were and having recently read this Performance Considerations page, Nancy may (or may not) be suffering performance hits unnecessarily.

Nancy is a lot more complex than tuning a SQL query (for example) where it is often easy to identify a 10-1000x reduction in processing/response time; I could probably think of dozens of possible suggestions, but without knowing where processing time is actually spent (and being able to measure changes), I could just be causing premature optimisation, or even making performance worse!

Are there any plans to implement a set of benchmarks or profiling tools? Or any suggestions for same? A few are mentioned on Sam Saffron's blog but I haven't got round to investigating anything in depth yet and whether they'd be suitable for profiling Nancy.

[Jevonius]

I've noticed a few more performance-related bits over the weekend - this is a bit of a brain dump...

By performance I usually consider memory usage as well as raw speed (this hasn't really been mentioned above). Thinking about the AppDomainAssemblyTypeScanner in particular while typing that - I have no idea what it really does (my spelunking hasn't taken me to it yet, I've only seen it in passing, so this might be wrong), but if it's loading everything it can find on start up that would be causing start-up to take longer, and the Nancy process to use more memory. Is it feasible to allow for optional explicit rather than implicit discovery? There are perhaps other areas that do something similar where explicit rather than implicit discovery would help performance - I say that with the awareness that this goes against SDHP :grin:

Diagnostics on by default I didn't even realise there was a diagnostics system and yet it's loaded by default (I really should read docs first rather than code...). With a bit of experimentation I've managed to disable it (#1518) but I'm not sure how much of a difference this really makes from a performance/memory usage perspective. With diagnostics enabled I can see that the SuperSimple ViewEngine is loaded - is it still loaded with diagnostics disabled? If diagnostics is the only thing that uses it, not loading it into memory would help performance/memory usage. Is the order it's displayed in /_Nancy/info relevant to request processing? If SuperSimple is shown above Razor for example (as in my case), does this mean SuperSimple is interrogated for every request before Razor [I've just re-read a trace page and think this is not the case]? Is it possible to specify a Engine order for those people using more than one engine? Similarly on that page, is TestingDiagnosticProvider always loaded because of the type scanner, even with Diagnostics disabled?

DefaultViewLocator and DefaultViewLocationConventions Having discovered the diagnostics page, I noticed on the trace page that for even a simple Razor view (with a view and a layout), the resolving looks like this:

 [DefaultViewResolver] Resolving view for 'DemoDetails', using view location conventions.
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/Demo/DemoDetails-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/Demo/DemoDetails'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/Demo/DemoDetails-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/Demo/DemoDetails'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/DemoDetails-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/DemoDetails'
 [DefaultViewResolver] View resolved at 'views/demo/DemoDetails'
 [DefaultViewFactory] Attempting to resolve view engine for view extension cshtml
 [DefaultViewFactory] Rendering view with view engine Nancy.ViewEngines.Razor.RazorViewEngine
 [DefaultViewResolver] Resolving view for '_Layout', using view location conventions.
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/Demo/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/Demo/_Layout'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/Demo/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/Demo/_Layout'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/_Layout'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/_Layout'
 [DefaultViewResolver] Attempting to locate view using convention 'views/Demo/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/Demo/_Layout'
 [DefaultViewResolver] Attempting to locate view using convention 'Demo/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'Demo/_Layout'
 [DefaultViewResolver] Attempting to locate view using convention 'views/_Layout-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/_Layout'
 [DefaultViewResolver] View resolved at 'views/_Layout'
 [DefaultViewResolver] Resolving view for '_ViewStart', using view location conventions.
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/Demo/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/Demo/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/Demo/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/Demo/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/demo/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'demo/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention 'views/Demo/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/Demo/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention 'Demo/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'Demo/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention 'views/_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention 'views/_ViewStart'
 [DefaultViewResolver] Attempting to locate view using convention '_ViewStart-en-GB'
 [DefaultViewResolver] Attempting to locate view using convention '_ViewStart'
 [DefaultViewResolver] No view could be resolved using the available view location conventions.

I was surprised that this was the same for every request - it turns out my understanding of StaticConfiguration.Cache.EnableRuntimeViewDiscovery was flawed! I've started thinking about some way to cache the view resolving too, not just the view discovery. I think this would probably mean changing the way ViewLocationConventions work though - perhaps by having a "GetCacheKey" method if caching is enabled or something? More thought needed... I have just found the View location conventions page on the Wiki. Again, maybe I should just read all the docs first :smile: It has given me an idea for a "Ways to tune Performance" page on the Wiki though, which would go through all of these points and give guidance for tuning performance of a site once you've got it all working (i.e improving performance by improving upon configuration defaults that users might not be aware of).

There's a secondary element here of adjusting the Razor ViewEngine so that it doesn't always try and find _ViewStart after the first try. No idea how you'd go about doing that at the moment though.

Module Lifecycle We've been using Nancy where I work for a while, and something had always bothered me when debugging and stepping through modules and it was only this afternoon I realised what it is - the (generally large, route-defining) Module constructor happens on every request, I think, even though generally only one of the routes is used. Is that correct? How 'expensive' is this, and is there a way to avoid it (make them singletons?) without affecting SDHP? Could this be done on a per-module basis by way of a "Lifestyle" attribute maybe, if you know your module is thread-safe?

Think that's probably enough for now. Sorry for the length, :beers: if you made it this far!

[prasannavl]

Would like to hear @thecodejunkie thoughts on the above. Its quite a detailed breakup on things that can be improved, and I seem to get the feeling that exceptions are thrown quite a bit in the pipeline. Also, Model Binding phase is another one that can be improved, but before getting into that, I'm very much interested in what the Nancy Team's thoughts are on the above.

[webcoyote]

@Jevonius mentioned "A task of 'improve performance' is significantly easier if there is an appropriate set of benchmarks available that enable identification of problem code and can be used to validate improvements."

How about the Tech Empower benchmarks, which show that NancyFX performing at only ~6% of the top-rated framework (on Windows) and 0.3% on Mono!

[donnyv]

Any movement on performance benchmarks?

[khellang]

There's https://github.com/TechEmpower/FrameworkBenchmarks/pull/1565. I've still not gotten the benchmarks to run properly on my machine. You could help out, @donnyv...

[thecodejunkie]

We continuously audit our code, no need for us to have a long running open issue for it

[jocull]

I know I’m necro’ing an old issue here. Is there any explaination about the module lifecycle mentioned above regarding performance? I’m digging into Nancy this week (hoping to pick it up at my shop!) and also noticed how the constructors redeclare the routes each time. This feels... heavy. Of course I haven’t measured it. There’s a lot of dynamics to wrap my head around.