Skip to content

improve performance of schema and position lookups during feature decoding #560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cportele wants to merge 1 commit into
base: master
Choose a base branch
from
Open

improve performance of schema and position lookups during feature decoding #560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 149 additions & 30 deletions ...atform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureEventHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,11 @@ default Optional<T> schema() {
return Optional.empty();
}

List<String> path = path();

if (path.isEmpty()) {
if (isPathEmpty()) {
return Optional.ofNullable(mapping().getTargetSchema());
}

List<T> targetSchemas =
isUseTargetPaths()
? mapping().getSchemasForTargetPath(path)
: mapping().getSchemasForSourcePath(path);
List<T> targetSchemas = schemasForPath();

if (targetSchemas.isEmpty()) {
// No mapping found for path
Expand All @@ -134,16 +129,11 @@ default int pos() {
return -1;
}

List<String> path = path();

if (path.isEmpty()) {
if (isPathEmpty()) {
return -1;
}

List<Integer> positions =
isUseTargetPaths()
? mapping().getPositionsForTargetPath(path)
: mapping().getPositionsForSourcePath(path);
List<Integer> positions = positionsForPath();

int schemaIndex = schemaIndex() > -1 ? schemaIndex() : positions.size() - 1;
if (positions.size() > schemaIndex) {
Expand All @@ -159,17 +149,12 @@ default List<Integer> parentPos() {
return List.of();
}

List<String> path = path();

if (path.isEmpty()) {
if (isPathEmpty()) {
return List.of();
}

// TODO: by target path?
List<List<Integer>> positions =
isUseTargetPaths()
? mapping().getParentPositionsForTargetPath(path)
: mapping().getParentPositionsForSourcePath(path);
List<List<Integer>> positions = parentPositionsForPath();

int schemaIndex = schemaIndex() > -1 ? schemaIndex() : positions.size() - 1;
if (positions.size() > schemaIndex) {
Expand All @@ -185,16 +170,11 @@ default List<T> parentSchemas() {
return ImmutableList.of();
}

List<String> path = path();

if (path.isEmpty()) {
if (isPathEmpty()) {
return ImmutableList.of();
}

List<List<T>> parentSchemas =
isUseTargetPaths()
? mapping().getParentSchemasForTargetPath(path)
: mapping().getParentSchemasForSourcePath(path);
List<List<T>> parentSchemas = parentSchemasForPath();

if (parentSchemas.isEmpty()) {
return ImmutableList.of();
Expand All @@ -204,6 +184,39 @@ default List<T> 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<T> schemasForPath() {
return isUseTargetPaths()
? mapping().getSchemasForTargetPath(path())
: mapping().getSchemasForSourcePath(path());
}

default List<Integer> positionsForPath() {
return isUseTargetPaths()
? mapping().getPositionsForTargetPath(path())
: mapping().getPositionsForSourcePath(path());
}

default List<List<Integer>> parentPositionsForPath() {
return isUseTargetPaths()
? mapping().getParentPositionsForTargetPath(path())
: mapping().getParentPositionsForSourcePath(path());
}

default List<List<T>> parentSchemasForPath() {
return isUseTargetPaths()
? mapping().getParentSchemasForTargetPath(path())
: mapping().getParentSchemasForSourcePath(path());
}

@Value.Lazy
default boolean isRequired() {
return schema().filter(T::isRequired).isPresent();
Expand All @@ -213,7 +226,8 @@ default boolean isRequired() {
interface ModifiableContext<T extends SchemaBase<T>, U extends SchemaMappingBase<T>>
extends Context<T, U> {

// 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();
Expand All @@ -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
Expand Down Expand Up @@ -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<T> pathMemo() {
PathMemo<T> 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<T> currentMemo() {
PathMemo<T> 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<T> schemasForPath() {
PathMemo<T> memo = currentMemo();
if (memo.schemas == null) {
memo.schemas =
memo.useTargetPaths
? mapping().getSchemasForTargetPath(memo.path)
: mapping().getSchemasForSourcePath(memo.path);
}
return memo.schemas;
}

@Override
default List<Integer> positionsForPath() {
PathMemo<T> memo = currentMemo();
if (memo.positions == null) {
memo.positions =
memo.useTargetPaths
? mapping().getPositionsForTargetPath(memo.path)
: mapping().getPositionsForSourcePath(memo.path);
}
return memo.positions;
}

@Override
default List<List<Integer>> parentPositionsForPath() {
PathMemo<T> memo = currentMemo();
if (memo.parentPositions == null) {
memo.parentPositions =
memo.useTargetPaths
? mapping().getParentPositionsForTargetPath(memo.path)
: mapping().getParentPositionsForSourcePath(memo.path);
}
return memo.parentPositions;
}

@Override
default List<List<T>> parentSchemasForPath() {
PathMemo<T> 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()
Expand Down Expand Up @@ -304,6 +404,8 @@ default boolean isRequired(T schema, List<T> parentSchemas) {

ModifiableContext<T, U> setPathTracker(FeaturePathTracker pathTracker);

ModifiableContext<T, U> setPathMemo(PathMemo<T> pathMemo);

ModifiableContext<T, U> setValue(String value);

ModifiableContext<T, U> setValueType(SchemaBase.Type valueType);
Expand Down Expand Up @@ -335,6 +437,23 @@ default boolean isRequired(T schema, List<T> parentSchemas) {
ModifiableContext<T, U> 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<T extends SchemaBase<T>> {
long version = Long.MIN_VALUE;
boolean useTargetPaths;
List<String> path;
List<T> schemas;
List<Integer> positions;
List<List<T>> parentSchemas;
List<List<Integer>> parentPositions;
}

// T createContext();

void onStart(V context);
Expand Down
16 changes: 16 additions & 0 deletions ...latform-features/src/main/java/de/ii/xtraplatform/features/domain/FeaturePathTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ public class FeaturePathTracker {

private final Joiner joiner;
private final List<String> 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);
Expand All @@ -34,13 +37,15 @@ public FeaturePathTracker(String separator) {
}

public void track(int depth) {
version++;
shorten(depth);
}

public void track(String localName, int depth) {
if (depth < 0) {
return;
}
version++;
shorten(depth);

track(localName);
Expand All @@ -56,14 +61,25 @@ private void shorten(final int depth) {
}

public void track(String localName) {
version++;
localPath.add(localName);
}

public void track(List<String> 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 "";
Expand Down
Loading