Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use fsnotify if using Directory and expose CompileTemplates #93

Merged
merged 2 commits into from
May 26, 2021

Conversation

zeripath
Copy link
Contributor

If we have set Options.IsDevelopment, if we can, use an fsnotify.Watcher to recompile the templates in a separate goroutine. This will significantly increase the performance of the Development server only recompiling if there is a change to the template files instead of recompiling on every request.

This will work well for renderers using the directory format however, if Options.Asset or Options.AssetFile is set then this performance gain will not be possible. However, now that compileTemplates properly locks the templates we can make the CompileTemplates function public and thus allow downstream users in that case the option to leave IsDevelopment false and instead recompile their templates, when and if, they detect a change within their assets.

Contains #92

Signed-off-by: Andrew Thornton art27@cantab.net

As a second step improving unrolled#90, only lock when absolutely necessary
and reconstruct functions to ensure that the current templates are
referenced in the helper func instead of a global reference.

Closes unrolled#91

Signed-off-by: Andrew Thornton <art27@cantab.net>
If setting IsDevelopment, if we can, use an FsWatcher to recompile the
templates in a separate goroutine. This should definitely increase the
performance of the Development server.

In order to make Asset based Renders have the same improvement - now
that render compilation properly locks the templates we can expose the
CompileTemplates function and allow downstream users to call this
independently when their templates need recompilation.

Contains unrolled#92

Signed-off-by: Andrew Thornton <art27@cantab.net>
@@ -399,17 +432,19 @@ func (r *Render) Data(w io.Writer, status int, v []byte) error {
func (r *Render) HTML(w io.Writer, status int, name string, binding interface{}, htmlOpt ...HTMLOptions) error {

// If we are in development mode, recompile the templates on every HTML request.
if r.opt.IsDevelopment {
r.compileTemplates()
r.lock.RLock() // rlock here because we're reading the hasWatcher
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the lock to read r.hasWatcher? Looks like it's only set within the CompileTemplates func.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeripath Looking again, the lock is probably a good idea since CompileTemplates is now exposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't get round to replying re: this - yes that was the reasoning.

@unrolled unrolled merged commit 549aa82 into unrolled:v1 May 26, 2021
@unrolled
Copy link
Owner

My apologies for the delay on merging this!

@zeripath zeripath deleted the use-fsnotify branch May 26, 2021 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants