diff --git a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureEventHandler.java b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureEventHandler.java index 0343c3fb2..2a2132855 100644 --- a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureEventHandler.java +++ b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureEventHandler.java @@ -106,16 +106,11 @@ default Optional schema() { return Optional.empty(); } - List path = path(); - - if (path.isEmpty()) { + if (isPathEmpty()) { return Optional.ofNullable(mapping().getTargetSchema()); } - List targetSchemas = - isUseTargetPaths() - ? mapping().getSchemasForTargetPath(path) - : mapping().getSchemasForSourcePath(path); + List targetSchemas = schemasForPath(); if (targetSchemas.isEmpty()) { // No mapping found for path @@ -134,16 +129,11 @@ default int pos() { return -1; } - List path = path(); - - if (path.isEmpty()) { + if (isPathEmpty()) { return -1; } - List positions = - isUseTargetPaths() - ? mapping().getPositionsForTargetPath(path) - : mapping().getPositionsForSourcePath(path); + List positions = positionsForPath(); int schemaIndex = schemaIndex() > -1 ? schemaIndex() : positions.size() - 1; if (positions.size() > schemaIndex) { @@ -159,17 +149,12 @@ default List parentPos() { return List.of(); } - List path = path(); - - if (path.isEmpty()) { + if (isPathEmpty()) { return List.of(); } // TODO: by target path? - List> positions = - isUseTargetPaths() - ? mapping().getParentPositionsForTargetPath(path) - : mapping().getParentPositionsForSourcePath(path); + List> positions = parentPositionsForPath(); int schemaIndex = schemaIndex() > -1 ? schemaIndex() : positions.size() - 1; if (positions.size() > schemaIndex) { @@ -185,16 +170,11 @@ default List parentSchemas() { return ImmutableList.of(); } - List path = path(); - - if (path.isEmpty()) { + if (isPathEmpty()) { return ImmutableList.of(); } - List> parentSchemas = - isUseTargetPaths() - ? mapping().getParentSchemasForTargetPath(path) - : mapping().getParentSchemasForSourcePath(path); + List> parentSchemas = parentSchemasForPath(); if (parentSchemas.isEmpty()) { return ImmutableList.of(); @@ -204,6 +184,39 @@ default List parentSchemas() { return parentSchemas.get(schemaIndex); } + // The lookups below back schema()/pos()/parentPos()/parentSchemas(). They are factored into + // their own methods so ModifiableContext can memoize them per tracked path: schema() etc. are + // @Value.Lazy, but @Value.Lazy is not cached on a @Value.Modifiable, so without memoization the + // same path is looked up again on every call. The defaults here are the unmemoized fallback. + + default boolean isPathEmpty() { + return path().isEmpty(); + } + + default List schemasForPath() { + return isUseTargetPaths() + ? mapping().getSchemasForTargetPath(path()) + : mapping().getSchemasForSourcePath(path()); + } + + default List positionsForPath() { + return isUseTargetPaths() + ? mapping().getPositionsForTargetPath(path()) + : mapping().getPositionsForSourcePath(path()); + } + + default List> parentPositionsForPath() { + return isUseTargetPaths() + ? mapping().getParentPositionsForTargetPath(path()) + : mapping().getParentPositionsForSourcePath(path()); + } + + default List> parentSchemasForPath() { + return isUseTargetPaths() + ? mapping().getParentSchemasForTargetPath(path()) + : mapping().getParentSchemasForSourcePath(path()); + } + @Value.Lazy default boolean isRequired() { return schema().filter(T::isRequired).isPresent(); @@ -213,7 +226,8 @@ default boolean isRequired() { interface ModifiableContext, U extends SchemaMappingBase> extends Context { - // TODO: default values are not cached by Modifiable + // a @Value.Default is not cached on a Modifiable, so create the value, store it via the + // setter and reuse that instance on subsequent calls @Value.Default default ModifiableCollectionMetadata metadata() { ModifiableCollectionMetadata collectionMetadata = ModifiableCollectionMetadata.create(); @@ -223,7 +237,8 @@ default ModifiableCollectionMetadata metadata() { return collectionMetadata; } - // TODO: default values are not cached by Modifiable + // a @Value.Default is not cached on a Modifiable, so create the value, store it via the + // setter and reuse that instance on subsequent calls @Value.Default default FeaturePathTracker pathTracker() { // when tracking target paths, if present, use path separator from flatten transformation in @@ -253,6 +268,91 @@ default String pathAsString() { return pathTracker().toStringWithDefaultSeparator(); } + // a @Value.Default is not cached on a Modifiable, so create the value, store it via the + // setter and reuse that instance on subsequent calls + @Value.Default + @Value.Auxiliary + default PathMemo pathMemo() { + PathMemo pathMemo = new PathMemo<>(); + + setPathMemo(pathMemo); + + return pathMemo; + } + + // Returns the path memo, resetting its cached lookups when the tracked path (or the + // target/source path mode) has changed since they were last computed. + private PathMemo currentMemo() { + PathMemo memo = pathMemo(); + long version = pathTracker().version(); + boolean useTargetPaths = isUseTargetPaths(); + + if (memo.version != version || memo.useTargetPaths != useTargetPaths) { + memo.version = version; + memo.useTargetPaths = useTargetPaths; + memo.path = pathTracker().asList(); + memo.schemas = null; + memo.positions = null; + memo.parentSchemas = null; + memo.parentPositions = null; + } + + return memo; + } + + @Override + default boolean isPathEmpty() { + return pathTracker().isEmpty(); + } + + @Override + default List schemasForPath() { + PathMemo memo = currentMemo(); + if (memo.schemas == null) { + memo.schemas = + memo.useTargetPaths + ? mapping().getSchemasForTargetPath(memo.path) + : mapping().getSchemasForSourcePath(memo.path); + } + return memo.schemas; + } + + @Override + default List positionsForPath() { + PathMemo memo = currentMemo(); + if (memo.positions == null) { + memo.positions = + memo.useTargetPaths + ? mapping().getPositionsForTargetPath(memo.path) + : mapping().getPositionsForSourcePath(memo.path); + } + return memo.positions; + } + + @Override + default List> parentPositionsForPath() { + PathMemo memo = currentMemo(); + if (memo.parentPositions == null) { + memo.parentPositions = + memo.useTargetPaths + ? mapping().getParentPositionsForTargetPath(memo.path) + : mapping().getParentPositionsForSourcePath(memo.path); + } + return memo.parentPositions; + } + + @Override + default List> parentSchemasForPath() { + PathMemo memo = currentMemo(); + if (memo.parentSchemas == null) { + memo.parentSchemas = + memo.useTargetPaths + ? mapping().getParentSchemasForTargetPath(memo.path) + : mapping().getParentSchemasForSourcePath(memo.path); + } + return memo.parentSchemas; + } + @Value.Lazy default boolean shouldSkip() { return schema().isEmpty() @@ -304,6 +404,8 @@ default boolean isRequired(T schema, List parentSchemas) { ModifiableContext setPathTracker(FeaturePathTracker pathTracker); + ModifiableContext setPathMemo(PathMemo pathMemo); + ModifiableContext setValue(String value); ModifiableContext setValueType(SchemaBase.Type valueType); @@ -335,6 +437,23 @@ default boolean isRequired(T schema, List parentSchemas) { ModifiableContext setCanonicalFeatureId(@Nullable String canonicalFeatureId); } + /** + * Per-context cache for the path-keyed lookups behind {@link Context#schema()}, {@link + * Context#pos()}, {@link Context#parentPos()} and {@link Context#parentSchemas()}. The lookups + * are recomputed whenever {@link #version} (the {@link FeaturePathTracker} version) or {@link + * #useTargetPaths} no longer matches; otherwise the cached values are reused. Mutable, single- + * threaded scratch state owned by one context instance. + */ + final class PathMemo> { + long version = Long.MIN_VALUE; + boolean useTargetPaths; + List path; + List schemas; + List positions; + List> parentSchemas; + List> parentPositions; + } + // T createContext(); void onStart(V context); diff --git a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeaturePathTracker.java b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeaturePathTracker.java index 5c4aff12b..a3f95f800 100644 --- a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeaturePathTracker.java +++ b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeaturePathTracker.java @@ -22,6 +22,9 @@ public class FeaturePathTracker { private final Joiner joiner; private final List localPath; + // bumped on every mutation so callers can cheaply detect a path change (e.g. lookup memoization) + // without rebuilding/comparing the path list + private long version; public FeaturePathTracker() { this.localPath = new ArrayList<>(64); @@ -34,6 +37,7 @@ public FeaturePathTracker(String separator) { } public void track(int depth) { + version++; shorten(depth); } @@ -41,6 +45,7 @@ public void track(String localName, int depth) { if (depth < 0) { return; } + version++; shorten(depth); track(localName); @@ -56,14 +61,25 @@ private void shorten(final int depth) { } public void track(String localName) { + version++; localPath.add(localName); } public void track(List path) { + version++; localPath.clear(); localPath.addAll(path); } + /** Monotonically increasing token that changes on every mutation of the tracked path. */ + public long version() { + return version; + } + + public boolean isEmpty() { + return localPath.isEmpty(); + } + @Override public String toString() { if (localPath.isEmpty()) return "";