Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

MVC precompilation design notes 5/12/2015 #2551

Closed
Eilon opened this issue May 12, 2015 · 10 comments
Closed

MVC precompilation design notes 5/12/2015 #2551

Eilon opened this issue May 12, 2015 · 10 comments

Comments

@Eilon
Copy link
Member

Eilon commented May 12, 2015

Today we had a design meeting with: @Alxandr @danroth27 @DamianEdwards @davidfowl @dougbu @lodejard @pranavkm @NTaylorMullen @ToddGrun @muratg

Please add your own thoughts as comments to this thread.

Agenda


Meeting notes

  1. Precompilation should not be enabled by default in project templates (it can exist, but should not be used in DTH scenarios)
  2. Precompilation is for:
    a. Server optimization (especially for large sites)
    b. Avoiding using a compiler on the server, which is good for security purposes
    c. Components with everything compiled in (including views) and ready to go
  3. Precompilation should not require any physical files on disk
  4. Precompilation behaviors:
    a. All view locators would always check for a file “on disk” (whether it’s physical or virtual)
    b. Precompilation would embed at least empty placeholder files into a virtual embedded file system
    c. You could also have a case where you embed the full files into the embedded file system, but also have precompilation enabled. You would then have updateable precompilation.
  5. We should use precompilation (or something else that’s similar! E.g. a Roslyn Analyzer?) to enable showing Razor errors in VS
  6. Question: Should any of these behaviors change while in DesignTimeHost (DTH)?
  7. Question: How do you enable precompilation? Today it’s a CompileModule. Should it be something else?
  8. Action item: Need to dig into exactly why the tag helper scenario is broken, and address that specifically.
  9. Action item: Should we remove Roslyn dependency from Razor? (Not interesting right now)
@Alxandr
Copy link
Contributor

Alxandr commented May 13, 2015

I'd like to add a few comments, some of which I didn't and some of which I did get to mention during said meeting, but didn't think I managed to explain properly. Some things are easier to do in writing without the time-constraint and added nervousness of suddenly being put in a conference call with the people who might have written most of MVC and DNX (it was great fun btw).

But before I begin I feel I should say that my standpoint is one of wanting to consume precompiled views, ie. serve them to the client. How these precompiled views are generated (as long as it's not obnoxiously hard), and how the tooling and or VS deals with all of this is outside the scope of what I'm about to discuss, because I have no idea how any of that works presently, and am in no way qualified to discuss the matter. So if there is anything in what I am writing that would not work well with tooling or such, by all means please point it out.

Also, in writing this, I am of the understanding that how precompiled views in Razor work today is up for discussion, and might be changed entirely. That includes how the views are precompiled, and how the are located/served by the application running, and any other aspects that deal with the idea of precompiled Razor views.

Today, when you in your HomeController.Index return View(), the request eventually reaches the RazorViewEngine, which has a list of search paths to search through for your view. This list consists of /Views/{1}/{0}.cshtml and /Views/Shared/{0}.cshtml (and also paths for Areas, but I'm ignoring them here for simplicity). The RazorViewEngine goes through these in order (inserting controller name and action name) and hands the path to a IRazorPageFactory defaulting to VirtualPathRazorPageFactory. The page factory then goes searching through the files on disk and the precompiled views for a view that matches the path (for instance /Views/Home/Index.cshtml), and makes sure the file isn't updated on disk. After doing all that it returns a IRazorView to the view engine, or null if no view that mached the path was found.

I suggest, that instead of having the view engine have one page factory, it can have a list of them (which can be added to or removed from by the end user application, typically during configuration of MVC and DI). One of these would be the VirtualPathRazorPageFactory and would work very much like the one already in existence. It would search for files in a virtual file system (which would normally be the disk) like it does today, and it would probably handle making sure that if the file is changed in said file system, a new version is compiled and served instead. In almost all manners, it would work exactly as the one already existing, except that any notion of precompiled views would be taken out. It would only deal with views that exist in .cshtml form in a (virtual or not) file system.

One of the boons presented by simply having a list of page factories (even without actually adding any other kind of page factories) is the ability to add more to the list. So, for instance, if you have 3 assemblies which have embedded .cshtml files, you can crate virtual file systems for each of them, and add them to the list of page factories in the order they are to be prioritized. This would also mean that you'd have control over the prioritizing of conflicting views, for instance if assembly A has a embedded .cshtml file named /Views/Home/Index.cshtml, while there is one with the same path in the application, you can order the page factories to decide which one "wins".

Secondly a second page factory should be added to MVC, something like PrecompiledRazorPageFactory. This page factory would do one simple thing, and that's return precompiled views. It would not care about file systems (virtual or not), and it would not do any checks to look for newer versions of the view.

By default, the two page factories that's used by MVC would look as following:

  1. A VirtualPathRazorPageFactory with it's virtual path set to point to the project directory in the actual file system.
  2. A PrecompiledRazorPageFactory.

Let's go through some of the scenarios with this setup. For the purpose of simplifying these explanations, I'm going to assume that in all of them there are two projects, A, and B. Both have the view Views/Home/Index.cshtml. Project A is the app we're running (with dnx . web), and only project B have turned on precompiling of views.

Scenario 1: Default setup, request Views/Home/Index.cshtml:

The engine starts by asking the VirtualPathRazorPageFactory for the view. The virtual path factory finds it on disk, compiles it, and returns it to the view engine, which in return sends it to the client.

If the view is changed on disk, the next request to the VirtualPathRazorPageFactory sees this, and recompiles.

If the file is deleted, the page factory sees this, and returns null, to let the next page factory handle the request, which would result in the precompiled view being served.

The point here to take home is the fact that the existence of a .cshtml file (by default) forces the view engine to ignore any precompiled views, and instead compile from sources during runtime. I feel this is in line with how DNX works in general, in that it prefers project references over package references, and builds sources at runtime.

Scenario 2: The order of the page factories are switched, so the PrecompiledRazorPageFactory is first. Same request.

At this point I think it's fairly obvious what is going to happen, but I'm going to go through it nonetheless, just to be thorough. Initially, the view returned will be the precompiled one, but the point is that no matter what you do to the .cshtml on disk, it won't matter. The precompiled view has precedence, so it will always be served.

Scenario 3: Developer wants to make sure that no views are compiled at runtime (for security or other reasons):

This suddenly becomes really easy. All you have to do is to remove the VirtualPathRazorPageFactory, and no .cshtml code will be compiled at runtime. If the file is not in the list of precompiled views, you will get a "view not found" error, just as you do today if there is no .cshtml file.

Scenario 4: Deployment to server

This comes down to a question of "do you want to be able to edit the views on the server?" If yes, then simply deploy without doing any precompilation. If you do not, compile with precompilation, and either make sure that the VirtualPathRazorPageFactory is removed, or that the .cshtml files is not deployed with your app (just like the .cs files aren't). This should preferably somehow be handled by dnu publish --no-source though.

Wrapping up

This post is probably the longest I've ever written on any GitHub issue, and it could probably be a blog post. It does not solve all of the problems agreed on in the meeting, nor was it intended to. This was mainly an attempt to communicate my thoughts after I had the time to go through some of Razor's sources, and with the ability to take time to write this all down properly.

Lastly, I'd like to point out point 4.a of the meeting notes, namely that "All view locators would always check for a file “on disk” (whether it’s physical or virtual)". Now, if this remains a requirement for using precompiled views in Razor (like it is today), it will probably be simplified in such a way that when you turn on precompilation, .cshtml files will automatically be embedded in the assembly for you, so you as a end developer doesn't have to think about the fact, but I still think adding .cshtml files, empty or not, either as embedded resources or actual files on the file system, when you already have enough metadata in the assembly to determine what views exist as precompiled views seems convoluted. It adds extra overhead to generating assemblies with precompiled views (if you for some reason decide to do this on your own, and not with the build in precompiler), and you to a larger degree need to know how these components interact. It's not something I feel very strongly about, but I'd still like to voice my concern. Now, my proposal is far from the only way to solve this (and some of the other problems), but I at least think it's a way to do so, and one which I feel is clean and highly extendable.

@lodejard
Copy link
Contributor

Hello @Alxandr, nice to meet you. And thanks again for the interest and participation!

As always with most problems there are several solutions which can work - this is probably one of those situations where we're looking at several perfectly valid approaches to the problem and weighing them against each other. Having more than one view factory could certainly be made to work, but I'm not sure it's really necessary. It might also be true that the way precompilation is currently supported at runtime could also be simplified. I'm thinking specifically of how I've seen other view engine support pre-compilation, so as always I'm biased in that way :). I'll try to walk though how it was done.

Setting aside for a moment the idea of precompilation or running without files on disk - a view engine in its default configuration has three moving parts:

  1. A file system abstraction it interfaces with to ask what exists, read the contents, and ask for change notifications
  2. The view factory itself that turns the appropriate source files into a compiled Type. Massive over-simplication here. :)
  3. A caching layer which - given input criteria and other "freshness" factors like file change notification - holds onto the previously generated Type to re-use instead of going through the work of reading/transforming/compiling the source.

Given that - precompilation was implemented without actually making the view factory itself aware of the feature at all. When the call was made to AddPrecompiledViews(Assembly[]) the following happened:

  1. It looked in those assemblies to find the view classes. The paths of files which were used to make those Types was present as attributes.
  2. It chained in a IFileProvider onto the options which made it look like those files existed. All that provider did was enumerate/return the files if asked, which were zero-bytes in length and had the same modified date as the Assembly. That was done using a standard CombinedFileProvider : IFileProvider and the existing physical file provider came before the PrecompilationSaysTheseFilesExist : IFileProvider.
  3. It pre-populated the view cache layer with an entry for each view Type.

That's all it took - the view location resolvers and view factory themselves had no idea precompilation was even a feature! When a view name was turned into a path it matched the normal file path patterns because it looked like it was present (even if it wasn't copied as a content file with the web app.) Then, when that path was instantiated, the pre-populated cache-hit meant it would use the already loaded Type. So at runtime it was literally a normal cache-hit - not a special case - the engine had no idea that the Type was precompiled view rather than a previously-compiled view.

@Tragetaschen
Copy link
Contributor

I just have more motivating scenarios for precompiled views:
2.
d. Embedded devices with frequent (compared to an internet server) restarts
e. .NET Native / AOT code generation

@danroth27 danroth27 added this to the 6.0.0-beta6 milestone May 14, 2015
@glen-84
Copy link

glen-84 commented Jun 2, 2015

Something like this could be huge in terms of enabling modular web applications (i.e. dropping a "Forum" or "Blog" module into an existing MVC web application.)

How difficult would it be to implement the following?

Opting in to pre-compilation:

In Login.cshtml

@using X
@class MyNs.LoginView (LoginViewModel model, SignInManager<ApplicationUser> signInManager)

<div>...</div>

When a class directive is present (with the format @class Namespace.ClassName (ctor parameters, ...)), the view is automatically compiled into a class with the specified name (in the specified namespace) with an appropriate constructor (with the specified arguments). The param list would be optional, in which case a default constructor would be used (or one with a generic object Model argument).

Returning a pre-compiled view from a controller:

In AccountController.cs

public IActionResult Login
{
    return new LoginView(model, signInManager);
}

You may also be able to swap out the view using DI, which is really useful if you want to customize a view in a referenced assembly ("module"). Before returning the instance, you could also call methods on it and/or register event handlers to alter the text stream to be rendered (f.e. you may replace something in the HTML).

Benefits:

  1. A clear and logical way to opt-in to pre-compilation.
  2. The ability to easily pass multiple arguments to a view.
  3. Strongly-typed views, meaning less magic strings and an easy way to reference views in other assemblies.
  4. Clean separation of file-system-based (runtime-compiled) views, and pre-compiled views. No need for trying to map a view name or path to a type. Usage of return View() remains the same.
  5. Less need for @inject and ViewBag. I have an idea to further reduce the need, but I won't mention it now.
  6. The benefits of pre-compiled views: Fast, embedded, no server-side compilation, etc.

Edit: I forgot to mention, this can be implemented without the parameter list (just @class MyNs.LoginView) if it would require too many changes to ModelState, etc.

@crozone
Copy link

crozone commented Jun 2, 2015

👍 @glen-84. +1 for strongly typed views, I do hate how views are still strongly tied to their physical path. Also makes passing in the view model and other models more logical, with less magic behind the scenes.

@ToddThomson
Copy link

My requirements for precompilation fall under 2(c) above. I also agree with 3. Under 4, I am unsure of having the requirement of 4b (it seems like an implementation requirement ), but want 4c ( although I am not completely sure how this would work - I guess there are scenarios where the compilerCache entry needs to be rebuilt due to imports or global changes and having the source would allow for re-compilation ).
The ability to package an MVC module with controllers, services and views ( and perhaps embedded css and js ) is something that I've wanted since the first release of MVC.

@Eilon
Copy link
Member Author

Eilon commented Jul 23, 2015

@pranavkm putting this on your plate.

@Alxandr
Copy link
Contributor

Alxandr commented Jul 23, 2015

If you need me to test stuff with regards to crazy things I tend to do with this stuff (like plugging F# and Razor), just let me know :)

@Eilon
Copy link
Member Author

Eilon commented Jul 23, 2015

@Alxandr I'm sure we will 😄

@pranavkm pranavkm removed the 1 - Ready label Aug 5, 2015
pranavkm added a commit that referenced this issue Aug 5, 2015
* Allow precompiled views to be served when source file does not exist in
  file system.
* Cache results for views that do not exist on disk.

Fixes #2462 and fixes #2796.
Partially addresses #2551
pranavkm added a commit that referenced this issue Aug 7, 2015
* Allow precompiled views to be served when source file does not exist in
  file system.
* Cache results for views that do not exist on disk.

Fixes #2462 and fixes #2796.
Partially addresses #2551
@Eilon Eilon removed the 2 - Working label Aug 14, 2015
@Eilon Eilon modified the milestones: Discussion, 6.0.0-beta7 Aug 14, 2015
@pranavkm
Copy link
Contributor

We're done with the work per these design notes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants