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

Further improve locking #92

Closed
wants to merge 1 commit into from
Closed

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented May 12, 2021

As a second step improving #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.

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>
@zeripath zeripath changed the title Further improve locking (RWMutex version) Further improve locking (RWMutex) May 12, 2021
@zeripath
Copy link
Contributor Author

Basically there are two options here.

This one uses the standard RWMutex instead of Atomics which allows us to keep the mutex around for use in other situations - e.g. if we want to/need to lock on other parts of the Render in future we're going to need to keep a mutex around.

@zeripath zeripath changed the title Further improve locking (RWMutex) Further improve locking May 12, 2021
@zeripath
Copy link
Contributor Author

I've closed #91 as I think this locking approach is better and allows us to use-fsnotify in the directory system.

zeripath added a commit to zeripath/render that referenced this pull request May 12, 2021
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>
@lunny
Copy link
Contributor

lunny commented May 13, 2021

It's better that the lock is only in the templates and not exposed to render.
The templates could have init, lookup and compile methods.

And if development is a compile-time setting, we could have two implementations for templates to be lock free in production mode.

unrolled pushed a commit that referenced this pull request May 26, 2021
* Further improve locking (RWMutex version)

As a second step improving #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 #91

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

* Use fsnotify if using Directory and expose CompileTemplates

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 #92

Signed-off-by: Andrew Thornton <art27@cantab.net>
@unrolled
Copy link
Owner

@zeripath Can this one be closed now?

@zeripath
Copy link
Contributor Author

yup can be closed!

@zeripath zeripath closed this May 26, 2021
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.

3 participants