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

Make HTML rendering deferred (IHtmlContent) #830

Closed
pranavkm opened this issue Jul 21, 2014 · 6 comments
Closed

Make HTML rendering deferred (IHtmlContent) #830

pranavkm opened this issue Jul 21, 2014 · 6 comments

Comments

@pranavkm
Copy link
Contributor

Currently TemplateRenderer.Render returns a string which gets converted to HtmlString. This causes it to produce a new StringWriter buffer. We could have it return a HelperResult instead - it's consumed in the same way, but would avoid the extra buffer allocation.

@dougbu
Copy link
Member

dougbu commented Aug 11, 2014

@pranavkm could you clarify the issue? Are you seeing poor performance numbers for a scenario involving templates read from disk?

Also

  • TemplateRenderer only creates the StringWriter instance when dealing with templates read from disk; the usual Func<IHtmlHelper, string> doesn't need anything more.
  • The Action<TextWriter> delegate would need the IView, ViewData and ViewContext in its closure; this might mess up the IView lifetime.
  • HelperResult would need to be moved into Microsoft.AspNet.Mvc.Core.
  • The Func<IHtmlHelper, string> default actions would need to be reworked significantly to operate on a TextWriter.

@pranavkm
Copy link
Contributor Author

I did not profile this in any way, but noticed this when I was looking at https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.Razor/RazorTextWriter.cs. The intent of introducing that type was to avoid creating large strings in memory.

  • It would be nice if we could simply write to ViewContext.Writer without returning strings, but that would break the API surface.
  • I guess some aspects of how the template gets rendered would change, but it might be worthwhile. That said, we certainly need profiling before making that claim.
  • We could leave default actions largely untouched, changing
if (defaultActions.TryGetValue(viewName, out defaultAction))
{
    var result = defaultAction(MakeHtmlHelper(_viewContext, _viewData));
    return new HelperResult(writer => writer.Write(result));
}

@danroth27 danroth27 added this to the 6.0.0-rc1 milestone Oct 13, 2014
@yishaigalatzer
Copy link
Contributor

If a template ever renders a large portion of text > 80K there is definitely a major LOH issue that will cause pauses and lots of CPU spent in GC

@yishaigalatzer yishaigalatzer modified the milestones: 6.0.0-rc1, 6.0.0-beta3 Jan 14, 2015
@yishaigalatzer
Copy link
Contributor

@NTaylorMullen please tie this to the TagHelper Content change when working with @sornaks

@sornaks
Copy link

sornaks commented Feb 6, 2015

Note to self: Move HelperResult to Razor.Runtime.

@danroth27 danroth27 modified the milestones: 6.0.0-beta4, 6.0.0-beta5 Mar 20, 2015
@danroth27 danroth27 modified the milestones: 6.0.0-beta6, 6.0.0-beta5 May 18, 2015
@Eilon Eilon modified the milestones: 6.0.0-beta6, 6.0.0-beta7 Jul 1, 2015
@sornaks sornaks modified the milestones: 6.0.0-beta6, 6.0.0-beta7 Jul 8, 2015
@Eilon Eilon removed the 1 - Ready label Jul 8, 2015
@danroth27 danroth27 modified the milestones: 6.0.0-beta7, 6.0.0-beta6 Jul 16, 2015
@danroth27 danroth27 changed the title TemplateRenderer should return a HelperResult (Action<TextWriter>) Make HTML rendering deferred (IHtmlContent) Jul 16, 2015
@sornaks
Copy link

sornaks commented Aug 14, 2015

f2540f9

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

6 participants