This is an automated email from the ASF dual-hosted git repository.
thiagohp pushed a commit to branch better-page-invalidation
in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
The following commit(s) were added to refs/heads/better-page-invalidation by
this push:
new db8a6f3fe TAP5-2742: handling page dependencies better (or at least
trying)
db8a6f3fe is described below
commit db8a6f3fee947b2246ce430da36c4910ce493a29
Author: Thiago H. de Paula Figueiredo <thi...@arsmachina.com.br>
AuthorDate: Thu May 25 18:55:50 2023 -0300
TAP5-2742: handling page dependencies better (or at least trying)
---
.../services/ComponentDependencyRegistryImpl.java | 67 ++++++--------------
.../services/ComponentInstantiatorSourceImpl.java | 11 +++-
.../internal/services/PageSourceImpl.java | 73 +++++++++++++++-------
.../apache/tapestry5/modules/TapestryModule.java | 7 ++-
.../pageload/PageClassloaderContextManager.java | 5 +-
.../PageClassloaderContextManagerImpl.java | 54 ++++++++++++----
6 files changed, 126 insertions(+), 91 deletions(-)
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java
index 92a41e34b..5231ff8a0 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java
@@ -52,6 +52,7 @@ import
org.apache.tapestry5.internal.structure.ComponentPageElement;
import org.apache.tapestry5.ioc.Orderable;
import org.apache.tapestry5.ioc.internal.util.ClasspathResource;
import org.apache.tapestry5.ioc.internal.util.InternalUtils;
+import org.apache.tapestry5.ioc.services.PerthreadManager;
import org.apache.tapestry5.json.JSONObject;
import org.apache.tapestry5.model.ComponentModel;
import org.apache.tapestry5.model.EmbeddedComponentModel;
@@ -68,6 +69,8 @@ import org.slf4j.Logger;
public class ComponentDependencyRegistryImpl implements
ComponentDependencyRegistry
{
+ private static final List<String> EMPTY_LIST = Collections.emptyList();
+
final private PageClassloaderContextManager pageClassloaderContextManager;
private static final String META_ATTRIBUTE =
"injectedComponentDependencies";
@@ -151,6 +154,13 @@ public class ComponentDependencyRegistryImpl implements
ComponentDependencyRegis
}
+ public void setupThreadCleanup(final PerthreadManager perthreadManager)
+ {
+ perthreadManager.addThreadCleanupCallback(() -> {
+ INVALIDATIONS_DISABLED.set(0);
+ });
+ }
+
@Override
public void register(Class<?> component)
{
@@ -584,14 +594,14 @@ public class ComponentDependencyRegistryImpl implements
ComponentDependencyRegis
// Protected just for testing
List<String> listen(List<String> resources)
{
- List<String> furtherDependents;
- if (INVALIDATIONS_DISABLED.get() > 0)
+ List<String> furtherDependents = EMPTY_LIST;
+ if (resources.isEmpty())
{
- furtherDependents = Collections.emptyList();
+ clear();
+ furtherDependents = EMPTY_LIST;
}
- else if (resources.isEmpty())
+ else if (INVALIDATIONS_DISABLED.get() > 0)
{
- clear();
furtherDependents = Collections.emptyList();
}
// Don't invalidate component dependency information when
@@ -613,12 +623,9 @@ public class ComponentDependencyRegistryImpl implements
ComponentDependencyRegis
}
clear(resource);
+
}
}
- else
- {
- furtherDependents = Collections.emptyList();
- }
return furtherDependents;
}
@@ -688,6 +695,10 @@ public class ComponentDependencyRegistryImpl implements
ComponentDependencyRegis
public void enableInvalidations()
{
INVALIDATIONS_DISABLED.set(INVALIDATIONS_DISABLED.get() - 1);
+ if (INVALIDATIONS_DISABLED.get() < 0)
+ {
+ INVALIDATIONS_DISABLED.set(0);
+ }
}
/**
@@ -848,44 +859,6 @@ public class ComponentDependencyRegistryImpl implements
ComponentDependencyRegis
}
}
-
- final private class InvalidationOnlyDependency
- {
-
- private String className;
- private String dependency;
-
- public InvalidationOnlyDependency(String className, String dependency)
- {
- super();
- this.className = className;
- this.dependency = dependency;
- }
-
- @Override
- public int hashCode()
- {
- final int prime = 31;
- int result = 1;
- result = prime * result + Objects.hash(className, dependency);
- return result;
- }
-
- @Override
- public boolean equals(Object obj)
- {
- if (this == obj)
- {
- return true;
- }
- if (!(obj instanceof InvalidationOnlyDependency)) {
- return false;
- }
- InvalidationOnlyDependency other = (InvalidationOnlyDependency)
obj;
- return Objects.equals(className, other.className) &&
Objects.equals(dependency, other.dependency);
- }
-
- }
private static final class Dependency
{
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
index bfafdeeb1..4d0fa42ea 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
@@ -341,8 +341,15 @@ public final class ComponentInstantiatorSourceImpl
implements ComponentInstantia
OPEN_INSTANTIATORS.get().add(className);
componentDependencyRegistry.disableInvalidations();
- PageClassloaderContext context =
pageClassloaderContextManager.get(className);
- componentDependencyRegistry.enableInvalidations();
+ PageClassloaderContext context;
+ try
+ {
+ context =
pageClassloaderContextManager.get(className);
+ }
+ finally
+ {
+ componentDependencyRegistry.enableInvalidations();
+ }
// Make sure the dependencies have been processed in
case
// there was some invalidation going on and they're
not there.
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
index 5d9dd16e5..ebf86306e 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
@@ -118,7 +118,21 @@ public class PageSourceImpl implements PageSource
public Page getPage(String canonicalPageName)
{
- return getPage(canonicalPageName, true);
+ if (!productionMode)
+ {
+ componentDependencyRegistry.disableInvalidations();
+ }
+ try
+ {
+ return getPage(canonicalPageName, true);
+ }
+ finally
+ {
+ if (!productionMode)
+ {
+ componentDependencyRegistry.enableInvalidations();
+ }
+ }
}
public Page getPage(String canonicalPageName, boolean
invalidateUnknownContext)
@@ -131,11 +145,6 @@ public class PageSourceImpl implements PageSource
// with all of its mutable construction-time state, is properly
published to other
// threads (at least, as I understand Brian Goetz's explanation, it
should be).
- if (!productionMode && invalidateUnknownContext)
- {
- componentDependencyRegistry.disableInvalidations();
- }
-
while (true)
{
SoftReference<Page> ref = pageCache.get(key);
@@ -144,10 +153,6 @@ public class PageSourceImpl implements PageSource
if (page != null)
{
- if (invalidateUnknownContext && !productionMode)
- {
- componentDependencyRegistry.enableInvalidations();
- }
return page;
}
@@ -184,11 +189,19 @@ public class PageSourceImpl implements PageSource
componentDependencyRegistry.clear(rootElement);
componentDependencyRegistry.register(rootElement);
PageClassloaderContext context =
pageClassloaderContextManager.get(className);
- if (invalidateUnknownContext && context.isUnknown())
+
+ if (context.isUnknown())
{
-
pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context);
- pageClassloaderContextManager.get(className);
- preprocessPageDependencies(className);
+ this.pageCache.remove(key);
+ if (invalidateUnknownContext)
+ {
+// pageClassloaderContextManager.invalidate(context);
+
pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context);
+ preprocessPageDependencies(className);
+ }
+// context.getClassNames().remove(className);
+ context.getClassNames().clear();
+ // Avoiding bad invalidations
return getPage(canonicalPageName, false);
}
}
@@ -224,18 +237,30 @@ public class PageSourceImpl implements PageSource
pageClassloaderContextManager.get(className);
for (String pageClassName : pageDependencies)
{
- pageClassloaderContextManager.get(pageClassName);
+ final PageClassloaderContext context =
pageClassloaderContextManager.get(pageClassName);
+ if (i == 1)
+ {
+ try
+ {
+ context.getClassLoader().loadClass(pageClassName);
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
}
}
- }
-
- private Class<?> getSuperclass(final String className,
PageClassloaderContext context)
- {
- try {
- return
context.getClassLoader().loadClass(className).getSuperclass();
- } catch (ClassNotFoundException e) {
- throw new RuntimeException(e);
- }
+
+ // TODO: remove
+// for (String pageClassName : pageDependencies)
+// {
+// try
+// {
+//
pageClassloaderContextManager.get(pageClassName).getClassLoader().loadClass(pageClassName);
+// } catch (ClassNotFoundException e)
+// {
+// throw new RuntimeException(e);
+// }
+// }
}
@PostInjection
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
index 5c57f9e57..b044523be 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
@@ -2791,9 +2791,10 @@ public final class TapestryModule
ComponentInstantiatorSource componentInstantiatorSource,
ComponentClassResolver componentClassResolver,
TemplateParser templateParser,
- ComponentTemplateLocator componentTemplateLocator)
+ ComponentTemplateLocator componentTemplateLocator,
+ PerthreadManager perthreadManager)
{
- ComponentDependencyRegistry componentDependencyRegistry =
+ ComponentDependencyRegistryImpl componentDependencyRegistry =
new ComponentDependencyRegistryImpl(
pageClassloaderContextManager,
componentInstantiatorSource.getProxyFactory().getPlasticManager(),
@@ -2803,6 +2804,8 @@ public final class TapestryModule
componentDependencyRegistry.listen(internalComponentInvalidationEventHub);
componentDependencyRegistry.listen(resourceChangeTracker);
componentDependencyRegistry.listen(componentTemplateSource.getInvalidationEventHub());
+ // TODO: remove
+ componentDependencyRegistry.setupThreadCleanup(perthreadManager);
return componentDependencyRegistry;
}
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
index 81b3b6167..375d0526f 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
@@ -85,9 +85,8 @@ public interface PageClassloaderContextManager
Class<?> getClassInstance(Class<?> clasz, String pageName);
/**
- * Invalidates the <code>unknown</code> context.
- * TODO: remove this
+ * Invalidates the "unknown" page classloader context context.
*/
void invalidateUnknownContext();
-
+
}
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
index 9c7cef88b..91241c661 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
@@ -82,14 +82,29 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
@Override
public void invalidateUnknownContext()
{
- for (PageClassloaderContext context : root.getChildren())
- {
- if (context.isUnknown())
+ synchronized (this) {
+ markAsNotInvalidatingContext();
+ for (PageClassloaderContext context : root.getChildren())
{
- componentDependencyRegistry.disableInvalidations();
- invalidateAndFireInvalidationEvents(context);
- componentDependencyRegistry.enableInvalidations();
- break;
+ if (context.isUnknown())
+ {
+// componentDependencyRegistry.disableInvalidations();
+// final Set<String> classNames = invalidate(context);
+// List<String> toInvalidate = new ArrayList<>();
+// for (String className : classNames)
+// {
+// if (root.findByClassName(className) == null)
+// {
+// toInvalidate.add(className);
+// }
+// }
+//
componentClassesInvalidationEventHub.fireInvalidationEvent(toInvalidate);
+// invalidationHub.fireInvalidationEvent(toInvalidate);
+//
+// componentDependencyRegistry.enableInvalidations();
+ invalidateAndFireInvalidationEvents(context);
+ break;
+ }
}
}
}
@@ -214,8 +229,10 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
context = root;
} else {
if (
- !componentDependencyRegistry.contains(className) &&
-
componentDependencyRegistry.getDependents(className).isEmpty())
+ !componentDependencyRegistry.contains(className)
+ // TODO: review this
+// &&
componentDependencyRegistry.getDependents(className).isEmpty()
+ )
{
context = unknownContextProvider.get();
// // Make sure you get a fresh version of the class before
processing its
@@ -306,7 +323,8 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
// Ensure non-page class is initialized in the correct
context and classloader.
// Pages get their own context and classloader, so this
initialization
// is both non-needed and a cause for an NPE if it happens.
- if (!componentClassResolver.isPage(className))
+ if (!componentClassResolver.isPage(className)
+ ||
componentDependencyRegistry.getDependencies(className,
DependencyType.USAGE).isEmpty())
{
loadClass(className, context);
}
@@ -547,21 +565,31 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
@Override
public void invalidateAndFireInvalidationEvents(PageClassloaderContext...
contexts) {
final Set<String> classNames = invalidate(contexts);
- INVALIDATING_CONTEXT.set(true);
+ markAsInvalidatingContext();
invalidate(classNames);
+ markAsNotInvalidatingContext();
+ }
+
+ private void markAsNotInvalidatingContext() {
+// System.out.println("XXXX Mark as NOT invalidating. Current: " +
INVALIDATING_CONTEXT.get());
INVALIDATING_CONTEXT.set(false);
}
+
+ private void markAsInvalidatingContext() {
+// System.out.println("XXXX Mark as invalidating. Current: " +
INVALIDATING_CONTEXT.get());
+ INVALIDATING_CONTEXT.set(true);
+ }
private void invalidate(Set<String> classesToInvalidate) {
if (!classesToInvalidate.isEmpty())
{
LOGGER.debug("Invalidating classes {}", classesToInvalidate);
- INVALIDATING_CONTEXT.set(true);
+ markAsInvalidatingContext();
final ArrayList<String> classesToInvalidateAsList = new
ArrayList<>(classesToInvalidate);
// TODO: do we really need both invalidation hubs to be invoked
here?
invalidationHub.fireInvalidationEvent(classesToInvalidateAsList);
componentClassesInvalidationEventHub.fireInvalidationEvent(classesToInvalidateAsList);
- INVALIDATING_CONTEXT.set(false);
+ markAsNotInvalidatingContext();
}
}
