Hello, Ben!

Your suggestion is great and I've implemented it in
https://github.com/apache/tapestry-5/commit/d6c4324215337b3bfb9ed8e62dd7174e0427410d
. It's some code that will be called a low number times during the
application runtime, so I decided to not try to optimize the
just-1-page-name case.
Cheers!

Th

On Sun, Jun 18, 2023 at 7:33 AM Ben Weidig <b...@netzgut.net> wrote:
>
> Hi Thiago,
>
> I like your implementation as a general solution better, as it solves the
> problem in a simple and straightforward way, whereas my approach would only
> provide the tools to build a solution by yourself.
>
> One thing I might have added is a helper method in the ReferenceType to
> create the most common contribution easier, maybe something like this
> pseudo/untested code:
>
> public enum ReferenceType {
>     SOFT,
>     STRONG;
>
>     public PageCachingReferenceTypeService forPages(String...
> canonicalPageNames) {
>         // Handle simplest case without allocating a HashSet
>         if (canonicalPageNames.length == 1) {
>             return pageName ->
> canonicalPageNames[0].equalsIgnoreCase(pageName) ? this : null;
>         }
>
>         // Prepare the page names for case-insensitive lookup
>         Set<String> pageNames = new HashSet<>(canonicalPageNames.length);
>         for (String pageName : canonicalPageNames) {
>             pageNames.add(pageName.toLowerCase());
>         }
>
>         return pageName -> pageNames.contains(pageName.toLowerCase()) ?
> this : null;
>     }
> }
>
> configuration.add("StrongPages", ReferenceType.STRONG.forPages("VeryLarge",
> "AndTheOtherLargePage"));
>
> Cheers
> Ben
>
> On Sun, Jun 18, 2023 at 12:11 AM Thiago H. de Paula Figueiredo <
> thiag...@gmail.com> wrote:
>
> > Hello!
> >
> > So I've just implemented what I suggested in Tapestry 5.8.3:
> > https://issues.apache.org/jira/browse/TAP5-2756. Example:
> >
> >     public static void contributePageCachingReferenceTypeService(
> >       OrderedConfiguration<PageCachingReferenceTypeService>
> > configuration) {
> >         configuration.add("VeryLarge", p -> p.equals("VeryLarge") ?
> > ReferenceType.STRONG : null);
> >     }
> >
> > This would cause the page named VeryLarge to be cached using a
> > regular, strong, non-garbage-collectable reference while leaving all
> > other pages cached with a soft, garbage-collectable reference.
> >
> > Ben, I find your idea interesting, but I believe it's a bit orthogonal
> > to what I did, both being able to coexist.
> >
> > On Thu, Jan 5, 2023 at 10:23 AM Thiago H. de Paula Figueiredo
> > <thiag...@gmail.com> wrote:
> > >
> > > Hello, everyone!
> > >
> > > I prefer Ben's idea of a thread or cron job to keep it fresh other
> > > than overriding a service, especially now that I'm working on
> > > something (smarter page invalidation, which is actually smarter
> > > invalidation of some key Tapestry caches) which changes that
> > > PageSourceImpl a bit and this override would likely break live class
> > > reloading. I'd have the cron/thread call ComponentSource.getPage()
> > > instead, since PageSource is an internal service and ComponentSource
> > > isn't.
> > >
> > > Another possibility is to introduce a configuration to tell whether
> > > PageSource should use regular references or soft ones, defaulting to
> > > current behavior. Or a new service to tell whether a specific page
> > > class should use a regular reference instead of a soft one. This would
> > > be more flexible.
> > >
> > > On Wed, Dec 28, 2022 at 6:55 AM Ben Weidig <b...@netzgut.net> wrote:
> > > >
> > > > Hi Geoff,
> > > >
> > > > I've read through the SoftReference documentation and as far as I
> > > > understand it the references do only get garbage-collected in case of
> > > > memory-pressure.
> > > > However, the behavior to keep recently used objects is only
> > encouraged, not
> > > > explicitly required.
> > > >
> > > > Looking over the source code, you mabye can replace PageSource with a
> > > > custom implementation that uses another caching implementation.
> > > > Something like this (untested) code maybe?
> > > >
> > > > package tapestry;
> > > >
> > > > import java.lang.ref.SoftReference;
> > > > import java.util.Map;
> > > >
> > > > import org.apache.tapestry5.commons.util.CollectionFactory;
> > > > import org.apache.tapestry5.internal.services.PageLoader;
> > > > import org.apache.tapestry5.internal.services.PageSourceImpl;
> > > > import org.apache.tapestry5.internal.structure.Page;
> > > > import
> > > >
> > org.apache.tapestry5.services.pageload.ComponentRequestSelectorAnalyzer;
> > > > import
> > org.apache.tapestry5.services.pageload.ComponentResourceSelector;
> > > >
> > > > public class CustomPageSourceImpl extends PageSourceImpl {
> > > >
> > > >     private PageLoader                       pageLoader;
> > > >     private ComponentRequestSelectorAnalyzer selectorAnalyzer;
> > > >
> > > >     public CustomPageSourceImpl(PageLoader pageLoader,
> > > > ComponentRequestSelectorAnalyzer selectorAnalyzer) {
> > > >         super(pageLoader, selectorAnalyzer);
> > > >         this.pageLoader = pageLoader;
> > > >         this.selectorAnalyzer = selectorAnalyzer;
> > > >
> > > >     }
> > > >
> > > >     private static final record CachedPageKey(String pageName,
> > > > ComponentResourceSelector selector) {
> > > >     }
> > > >
> > > >     private final Map<CachedPageKey, Object> pageCache =
> > > > CollectionFactory.newConcurrentMap();
> > > >
> > > >     public Page getPage(String canonicalPageName)
> > > >     {
> > > >         var selector = selectorAnalyzer.buildSelectorForRequest();
> > > >
> > > >         var key = new CachedPageKey(canonicalPageName, selector);
> > > >
> > > >         while (true)
> > > >         {
> > > >             Object cachedObject = pageCache.get(key);
> > > >
> > > >             Page page = null;
> > > >             if (cachedObject instanceof SoftReference<?> ref) {
> > > >                 page = ref == null ? null : (Page) ref.get();
> > > >             } else {
> > > >                 page = (Page) cachedObject;
> > > >             }
> > > >
> > > >             if (page != null)
> > > >             {
> > > >                 return page;
> > > >             }
> > > >             // In rare race conditions, we may see the same page loaded
> > > > multiple times across
> > > >             // different threads. The last built one will "evict" the
> > > > others from the page cache,
> > > >             // and the earlier ones will be GCed.
> > > >
> > > >             page = pageLoader.loadPage(canonicalPageName, selector);
> > > >
> > > >             // TODO: Decide here if how you want to store the Page
> > > >
> > > >             Object cacheValue = new SoftReference<Page>(page);
> > > >
> > > >             pageCache.put(key, cacheValue);
> > > >         }
> > > >     }
> > > > }
> > > >
> > > > I'm not sure what the implications are if a page is kept forever, but
> > as a
> > > > SoftReference isn't guaranteed to be garbage-collected, I don't see an
> > > > immediate downside, except needing more memory.
> > > >
> > > > Alternatively you could trigger the page with a cron job to keep it
> > > > "fresh", but an overriden service is the more robust solution in my
> > opinion.
> > > >
> > > > Cheers
> > > > Ben
> > > >
> > > > On Tue, Dec 27, 2022 at 3:24 PM JumpStart <
> > > > geoff.callender.jumpst...@gmail.com> wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I have one page in my production app which takes a long time to load
> > -
> > > > > minimum 18 seconds. Once loaded, it is very quick to request. But
> > from time
> > > > > to time throughout the day, when this page is requested Tapestry
> > decides it
> > > > > has to reload it. I presume this is because the page cache uses
> > > > > SoftReference (PageSourceImpl.pageCache) and the page has been
> > garbage
> > > > > collected. For the unlucky user, they have to wait an unbearably
> > long time.
> > > > > Sometimes when the system is under load the request can even time
> > out. Is
> > > > > there a simple, reliable, safe way to prevent it being garbage
> > collected?
> > > > > Or have I misunderstood what’s going on?
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Geoff
> > > > >
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
> > > > > For additional commands, e-mail: users-h...@tapestry.apache.org
> > > > >
> > > > >
> > >
> > >
> > >
> > > --
> > > Thiago
> >
> >
> >
> > --
> > Thiago
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
> > For additional commands, e-mail: users-h...@tapestry.apache.org
> >
> >



-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org

Reply via email to