-
John Hawthorn authored
Previously I attempted to de-dup and cache any FileSystemResolver under the same path. This created issues because each Resolver caches its templates. Those cached templates only get compiled once into whatever compiled_method_container is passed on its first render. So separate compiled_method_containers must have different copies of a Template, and therefore separate copies of each Resolvers. I tried a few approaches where I would have a separate cache of resolvers per-compiled-method-container, but I wasn't really successful as in most of the places we performed the casting we did not know or have access to the view context class and therefore compiled method container (it isn't stored in the LookupContext or ViewPath) and trying to add that relating created incompatibilities in some not-quite-public-but-still-widely used APIs for using ActionView directly. This approach instead limits where we perform the de-dup and cache, to only descendants of ActionView::ViewPaths. These can be assumed to also inherit from ActionView::Rendering (I think ActionController::API is an exception here, which should be addressed in a future refactor) which means that they should all share the same compiled_method_container and therefore can all be cached safely together. This should continue to ensure we don't leak memory from {append,prepend}_view_path at runtime, while maintaining compatibility with some of the more specialized uses of ActionView::Base as a standalone API. However this leaves open the possibility (which has always existed) of using that standalone API in a "leaky" way. * Invoke callback only once building view Resolvers Previously we would run the callback after each resolver built. We only need to possibly recreate the watcher once all new resolvers have been instantiated. Additionally this now only invokes the callback when a new resolver has actually been built, if one was found in the cache no work is needed. Co-authored-by:
Rafael Mendonça França <rafael@franca.dev>
John Hawthorn authoredPreviously I attempted to de-dup and cache any FileSystemResolver under the same path. This created issues because each Resolver caches its templates. Those cached templates only get compiled once into whatever compiled_method_container is passed on its first render. So separate compiled_method_containers must have different copies of a Template, and therefore separate copies of each Resolvers. I tried a few approaches where I would have a separate cache of resolvers per-compiled-method-container, but I wasn't really successful as in most of the places we performed the casting we did not know or have access to the view context class and therefore compiled method container (it isn't stored in the LookupContext or ViewPath) and trying to add that relating created incompatibilities in some not-quite-public-but-still-widely used APIs for using ActionView directly. This approach instead limits where we perform the de-dup and cache, to only descendants of ActionView::ViewPaths. These can be assumed to also inherit from ActionView::Rendering (I think ActionController::API is an exception here, which should be addressed in a future refactor) which means that they should all share the same compiled_method_container and therefore can all be cached safely together. This should continue to ensure we don't leak memory from {append,prepend}_view_path at runtime, while maintaining compatibility with some of the more specialized uses of ActionView::Base as a standalone API. However this leaves open the possibility (which has always existed) of using that standalone API in a "leaky" way. * Invoke callback only once building view Resolvers Previously we would run the callback after each resolver built. We only need to possibly recreate the watcher once all new resolvers have been instantiated. Additionally this now only invokes the callback when a new resolver has actually been built, if one was found in the cache no work is needed. Co-authored-by:
Rafael Mendonça França <rafael@franca.dev>
Loading