Hello!
Your're right, overriding an internal service is usually an invitation for
trouble with any bigger Tapestry update.
Still, personally, I prefer a service override over cron jobs triggering
things behind the scenes.
Especially since it's one of Tapestry's greatest features to replace almost
any part of it as needed, so I can break all the things :-D
Making the caching style more configurable might be a nice addition to
provide more freedom.
However, my first thought was a little bit more detached from
"SoftReference or not".
Something trivial like this:
interface PageCacheService {
Page getPage(String canonicalPageName, ComponentResourceSelector
selector);
void setPage(Page page, String canonicalPageName,
ComponentResourceSelector selector);
}
This way, any type of caching could be used, like a dedicated caching
library like EhCache.
The org.apache.tapestry5.internal.services.PageSourceImpl.CachedPageKey
could be exposed to be used instead of dedicated arguments for
canonicalPageName and selector.
The default implementation could be a simple wrapper around a
Map<CachedPageKey, SoftReference<Page>>, just like the one used now.
Cheers
On Thu, Jan 5, 2023 at 2:23 PM Thiago H. de Paula Figueiredo <
[email protected]> 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 <[email protected]> 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 <
> > [email protected]> 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: [email protected]
> > > For additional commands, e-mail: [email protected]
> > >
> > >
>
>
>
> --
> Thiago
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>
