Skip to content
  • John Hawthorn's avatar
    8a997600
    Only de-dup/cache FileSystemResolvers in ViewPaths · 8a997600
    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: default avatarRafael Mendonça França <rafael@franca.dev>
    8a997600
    Only de-dup/cache FileSystemResolvers in ViewPaths
    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: default avatarRafael Mendonça França <rafael@franca.dev>
Loading