From e3750370df3be1413b1526668cbee60dc2a39f03 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 30 Apr 2023 20:22:23 +0800 Subject: [PATCH] Use globally shared HTMLRender (#24436) The old `HTMLRender` is not ideal. 1. It shouldn't be initialized multiple times, it consumes a lot of memory and is slow. 2. It shouldn't depend on short-lived requests, the `WatchLocalChanges` needs a long-running context. 3. It doesn't make sense to use FuncsMap slice. HTMLRender was designed to only work for GItea's specialized 400+ templates, so it's good to make it a global shared instance. --- modules/context/context.go | 2 +- modules/context/package.go | 2 +- modules/templates/helper.go | 10 +++----- modules/templates/htmlrenderer.go | 37 +++++++++++++++++------------- modules/templates/mailer.go | 7 +++--- routers/api/v1/misc/markup_test.go | 2 +- routers/init.go | 2 +- routers/install/install.go | 2 +- routers/install/routes.go | 2 +- routers/web/base.go | 2 +- routers/web/web.go | 3 ++- 11 files changed, 37 insertions(+), 34 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index 702da8a96..cd7fcebe5 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -677,7 +677,7 @@ func getCsrfOpts() CsrfOptions { // Contexter initializes a classic context for a request. func Contexter(ctx context.Context) func(next http.Handler) http.Handler { - _, rnd := templates.HTMLRenderer(ctx) + rnd := templates.HTMLRenderer() csrfOpts := getCsrfOpts() if !setting.IsProd { CsrfTokenRegenerationInterval = 5 * time.Second // in dev, re-generate the tokens more aggressively for debug purpose diff --git a/modules/context/package.go b/modules/context/package.go index 2a0159eb5..6c418b316 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -131,7 +131,7 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { // PackageContexter initializes a package context for a request. func PackageContexter(ctx gocontext.Context) func(next http.Handler) http.Handler { - _, rnd := templates.HTMLRenderer(ctx) + rnd := templates.HTMLRenderer() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { ctx := Context{ diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 20261eb95..4abd94d46 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -10,7 +10,6 @@ import ( "html" "html/template" "net/url" - "regexp" "strings" "time" @@ -26,12 +25,9 @@ import ( "code.gitea.io/gitea/services/gitdiff" ) -// Used from static.go && dynamic.go -var mailSubjectSplit = regexp.MustCompile(`(?m)^-{3,}[\s]*$`) - // NewFuncMap returns functions for injecting to templates -func NewFuncMap() []template.FuncMap { - return []template.FuncMap{map[string]interface{}{ +func NewFuncMap() template.FuncMap { + return map[string]interface{}{ "DumpVar": dumpVar, // ----------------------------------------------------------------- @@ -192,7 +188,7 @@ func NewFuncMap() []template.FuncMap { "FilenameIsImage": FilenameIsImage, "TabSizeClass": TabSizeClass, - }} + } } // Safe render raw as HTML diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go index 2cecef5f8..d60be8872 100644 --- a/modules/templates/htmlrenderer.go +++ b/modules/templates/htmlrenderer.go @@ -6,7 +6,6 @@ package templates import ( "bufio" "bytes" - "context" "errors" "fmt" "io" @@ -15,24 +14,29 @@ import ( "regexp" "strconv" "strings" + "sync" "sync/atomic" texttemplate "text/template" "code.gitea.io/gitea/modules/assetfs" + "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates/scopedtmpl" "code.gitea.io/gitea/modules/util" ) -var rendererKey interface{} = "templatesHtmlRenderer" - type TemplateExecutor scopedtmpl.TemplateExecutor type HTMLRender struct { templates atomic.Pointer[scopedtmpl.ScopedTemplate] } +var ( + htmlRender *HTMLRender + htmlRenderOnce sync.Once +) + var ErrTemplateNotInitialized = errors.New("template system is not initialized, check your log for errors") func (h *HTMLRender) HTML(w io.Writer, status int, name string, data interface{}) error { @@ -55,14 +59,14 @@ func (h *HTMLRender) TemplateLookup(name string) (TemplateExecutor, error) { return nil, ErrTemplateNotInitialized } - return tmpls.Executor(name, NewFuncMap()[0]) + return tmpls.Executor(name, NewFuncMap()) } func (h *HTMLRender) CompileTemplates() error { assets := AssetFS() extSuffix := ".tmpl" tmpls := scopedtmpl.NewScopedTemplate() - tmpls.Funcs(NewFuncMap()[0]) + tmpls.Funcs(NewFuncMap()) files, err := ListWebTemplateAssetNames(assets) if err != nil { return nil @@ -86,20 +90,21 @@ func (h *HTMLRender) CompileTemplates() error { return nil } -// HTMLRenderer returns the current html renderer for the context or creates and stores one within the context for future use -func HTMLRenderer(ctx context.Context) (context.Context, *HTMLRender) { - if renderer, ok := ctx.Value(rendererKey).(*HTMLRender); ok { - return ctx, renderer - } +// HTMLRenderer init once and returns the globally shared html renderer +func HTMLRenderer() *HTMLRender { + htmlRenderOnce.Do(initHTMLRenderer) + return htmlRender +} +func initHTMLRenderer() { rendererType := "static" if !setting.IsProd { rendererType = "auto-reloading" } - log.Log(1, log.DEBUG, "Creating "+rendererType+" HTML Renderer") + log.Debug("Creating %s HTML Renderer", rendererType) - renderer := &HTMLRender{} - if err := renderer.CompileTemplates(); err != nil { + htmlRender = &HTMLRender{} + if err := htmlRender.CompileTemplates(); err != nil { p := &templateErrorPrettier{assets: AssetFS()} wrapFatal(p.handleFuncNotDefinedError(err)) wrapFatal(p.handleUnexpectedOperandError(err)) @@ -107,14 +112,14 @@ func HTMLRenderer(ctx context.Context) (context.Context, *HTMLRender) { wrapFatal(p.handleGenericTemplateError(err)) log.Fatal("HTMLRenderer CompileTemplates error: %v", err) } + if !setting.IsProd { - go AssetFS().WatchLocalChanges(ctx, func() { - if err := renderer.CompileTemplates(); err != nil { + go AssetFS().WatchLocalChanges(graceful.GetManager().ShutdownContext(), func() { + if err := htmlRender.CompileTemplates(); err != nil { log.Error("Template error: %v\n%s", err, log.Stack(2)) } }) } - return context.WithValue(ctx, rendererKey, renderer), renderer } func wrapFatal(msg string) { diff --git a/modules/templates/mailer.go b/modules/templates/mailer.go index 280ac0e58..ac1715fcd 100644 --- a/modules/templates/mailer.go +++ b/modules/templates/mailer.go @@ -6,6 +6,7 @@ package templates import ( "context" "html/template" + "regexp" "strings" texttmpl "text/template" @@ -14,6 +15,8 @@ import ( "code.gitea.io/gitea/modules/setting" ) +var mailSubjectSplit = regexp.MustCompile(`(?m)^-{3,}\s*$`) + // mailSubjectTextFuncMap returns functions for injecting to text templates, it's only used for mail subject func mailSubjectTextFuncMap() texttmpl.FuncMap { return texttmpl.FuncMap{ @@ -55,9 +58,7 @@ func Mailer(ctx context.Context) (*texttmpl.Template, *template.Template) { bodyTemplates := template.New("") subjectTemplates.Funcs(mailSubjectTextFuncMap()) - for _, funcs := range NewFuncMap() { - bodyTemplates.Funcs(funcs) - } + bodyTemplates.Funcs(NewFuncMap()) assetFS := AssetFS() refreshTemplates := func() { diff --git a/routers/api/v1/misc/markup_test.go b/routers/api/v1/misc/markup_test.go index 301f51eea..32de2956b 100644 --- a/routers/api/v1/misc/markup_test.go +++ b/routers/api/v1/misc/markup_test.go @@ -30,7 +30,7 @@ const ( ) func createContext(req *http.Request) (*context.Context, *httptest.ResponseRecorder) { - _, rnd := templates.HTMLRenderer(req.Context()) + rnd := templates.HTMLRenderer() resp := httptest.NewRecorder() c := &context.Context{ Req: req, diff --git a/routers/init.go b/routers/init.go index 2c26bb5b0..358922b1a 100644 --- a/routers/init.go +++ b/routers/init.go @@ -175,7 +175,7 @@ func GlobalInitInstalled(ctx context.Context) { // NormalRoutes represents non install routes func NormalRoutes(ctx context.Context) *web.Route { - ctx, _ = templates.HTMLRenderer(ctx) + _ = templates.HTMLRenderer() r := web.NewRoute() r.Use(common.ProtocolMiddlewares()...) diff --git a/routers/install/install.go b/routers/install/install.go index 8f8656230..c838db658 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -55,7 +55,7 @@ func getSupportedDbTypeNames() (dbTypeNames []map[string]string) { // Init prepare for rendering installation page func Init(ctx goctx.Context) func(next http.Handler) http.Handler { - _, rnd := templates.HTMLRenderer(ctx) + rnd := templates.HTMLRenderer() dbTypeNames := getSupportedDbTypeNames() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { diff --git a/routers/install/routes.go b/routers/install/routes.go index 9c7420c59..88c7e9969 100644 --- a/routers/install/routes.go +++ b/routers/install/routes.go @@ -66,7 +66,7 @@ func installRecovery(ctx goctx.Context) func(next http.Handler) http.Handler { if !setting.IsProd { store["ErrorMsg"] = combinedErr } - _, rnd := templates.HTMLRenderer(ctx) + rnd := templates.HTMLRenderer() err = rnd.HTML(w, http.StatusInternalServerError, "status/500", templates.BaseVars().Merge(store)) if err != nil { log.Error("%v", err) diff --git a/routers/web/base.go b/routers/web/base.go index b5d7a737b..73607cad0 100644 --- a/routers/web/base.go +++ b/routers/web/base.go @@ -120,7 +120,7 @@ func (d *dataStore) GetData() map[string]interface{} { // RecoveryWith500Page returns a middleware that recovers from any panics and writes a 500 and a log if so. // This error will be created with the gitea 500 page. func RecoveryWith500Page(ctx goctx.Context) func(next http.Handler) http.Handler { - _, rnd := templates.HTMLRenderer(ctx) + rnd := templates.HTMLRenderer() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { defer func() { diff --git a/routers/web/web.go b/routers/web/web.go index bb2442fec..e63add51f 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -114,7 +114,8 @@ func Routes(ctx gocontext.Context) *web.Route { routes.RouteMethods("/apple-touch-icon.png", "GET, HEAD", misc.StaticRedirect("/assets/img/apple-touch-icon.png")) routes.RouteMethods("/favicon.ico", "GET, HEAD", misc.StaticRedirect("/assets/img/favicon.png")) - ctx, _ = templates.HTMLRenderer(ctx) + _ = templates.HTMLRenderer() + common := []any{ common.Sessioner(), RecoveryWith500Page(ctx),