Skip to content

Commit

Permalink
PathMappings optimizations (#9055)
Browse files Browse the repository at this point in the history 
* Avoid iterations if only ServletPathSpec instances
* Avoid tests for empty mappings.
* Better reset implementation
* More test coverage
  • Loading branch information
@gregw
gregw committed Dec 20, 2022
1 parent 99b1b1e commit 4916377
Show file tree
Hide file tree
Showing 3 changed files with 267 additions and 46 deletions.
23 changes: 23 additions & 0 deletions jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MappedResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,34 @@ public class MappedResource<E> implements Comparable<MappedResource<E>>
{
private final PathSpec pathSpec;
private final E resource;
private final MatchedResource<E> preMatched;

public MappedResource(PathSpec pathSpec, E resource)
{
this.pathSpec = pathSpec;
this.resource = resource;

MatchedResource<E> matched;
switch (pathSpec.getGroup())
{
case ROOT:
matched = new MatchedResource<>(resource, pathSpec, pathSpec.matched("/"));
break;
case EXACT:
matched = new MatchedResource<>(resource, pathSpec, pathSpec.matched(pathSpec.getDeclaration()));
break;
default:
matched = null;
}
this.preMatched = matched;
}

/**
* @return A pre match {@link MatchedResource} for ROOT and EXACT matches, else null;
*/
public MatchedResource<E> getPreMatched()
{
return preMatched;
}

/**
Expand Down
191 changes: 163 additions & 28 deletions jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Predicate;
Expand All @@ -42,11 +45,13 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
private static final Logger LOG = LoggerFactory.getLogger(PathMappings.class);
private final Set<MappedResource<E>> _mappings = new TreeSet<>(Comparator.comparing(MappedResource::getPathSpec));

/**
* When _orderIsSignificant is true, the order of the MappedResources is significant and a match needs to be iteratively
* tried against each mapping (ordered by group then add order) to find the first that matches.
*/
private boolean _orderIsSignificant;
private boolean _optimizedExact = true;
private final Index.Mutable<MappedResource<E>> _exactMap = new Index.Builder<MappedResource<E>>()
.caseSensitive(true)
.mutable()
.build();
private final Map<String, MappedResource<E>> _exactMap = new HashMap<>();
private boolean _optimizedPrefix = true;
private final Index.Mutable<MappedResource<E>> _prefixMap = new Index.Builder<MappedResource<E>>()
.caseSensitive(true)
Expand All @@ -58,6 +63,9 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
.mutable()
.build();

private MappedResource<E> _servletRoot;
private MappedResource<E> _servletDefault;

@Override
public String dump()
{
Expand Down Expand Up @@ -86,6 +94,12 @@ public void reset()
_mappings.clear();
_prefixMap.clear();
_suffixMap.clear();
_optimizedExact = true;
_optimizedPrefix = true;
_optimizedSuffix = true;
_orderIsSignificant = false;
_servletRoot = null;
_servletDefault = null;
}

public void removeIf(Predicate<MappedResource<E>> predicate)
Expand Down Expand Up @@ -121,31 +135,117 @@ public List<MatchedResource<E>> getMatchedList(String path)
*/
public List<MappedResource<E>> getMatches(String path)
{
if (_mappings.isEmpty())
return Collections.emptyList();

boolean isRootPath = "/".equals(path);

List<MappedResource<E>> ret = new ArrayList<>();
// Iterator over all the mapping, adding only those that match.
List<MappedResource<E>> matches = null;
for (MappedResource<E> mr : _mappings)
{
switch (mr.getPathSpec().getGroup())
{
case ROOT:
if (isRootPath)
ret.add(mr);
{
if (matches == null)
matches = new ArrayList<>();
matches.add(mr);
}
break;
case DEFAULT:
if (isRootPath || mr.getPathSpec().matched(path) != null)
ret.add(mr);
{
if (matches == null)
matches = new ArrayList<>();
matches.add(mr);
}
break;
default:
if (mr.getPathSpec().matched(path) != null)
ret.add(mr);
{
if (matches == null)
matches = new ArrayList<>();
matches.add(mr);
}
break;
}
}
return ret;
return matches == null ? Collections.emptyList() : matches;
}

/**
* <p>Find the best single match for a path.</p>
* <p>The match may be found by optimized direct lookups when possible, otherwise all mappings
* are iterated over and the first match returned</p>
* @param path The path to match
* @return A {@link MatchedResource} instance or null if no mappings matched.
* @see #getMatchedIteratively(String)
*/
public MatchedResource<E> getMatched(String path)
{
if (_mappings.isEmpty())
return null;

// If order is significant, then we need to match by iterating over all mappings.
if (_orderIsSignificant)
return getMatchedIteratively(path);

// Otherwise, we can try optimized matches against each group

// Try a root match
if (_servletRoot != null && "/".equals(path))
return _servletRoot.getPreMatched();

// try an exact match
MappedResource<E> exact = _exactMap.get(path);
if (exact != null)
return exact.getPreMatched();

// Try a prefix match
MappedResource<E> prefix = _prefixMap.getBest(path);
if (prefix != null)
{
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
}

// Try a suffix match
if (!_suffixMap.isEmpty())
{
int i = Math.max(0, path.lastIndexOf("/"));
// Loop through each suffix mark
// Input is "/a.b.c.foo"
// Loop 1: "b.c.foo"
// Loop 2: "c.foo"
// Loop 3: "foo"
while ((i = path.indexOf('.', i + 1)) > 0)
{
prefix = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (prefix == null)
continue;

MatchedPath matchedPath = prefix.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
}
}

if (_servletDefault != null)
return new MatchedResource<>(_servletDefault.getResource(), _servletDefault.getPathSpec(), _servletDefault.getPathSpec().matched(path));

return null;
}

/**
* <p>Iterate over all mappings, returning the first that matches.</p>
* @param path The path to match.
* @return A {@link MatchedResource} instance or null if no mappings matched.
* @see #getMatched(String)
*/
private MatchedResource<E> getMatchedIteratively(String path)
{
MatchedPath matchedPath;
PathSpecGroup lastGroup = null;
Expand Down Expand Up @@ -173,19 +273,9 @@ public MatchedResource<E> getMatched(String path)
{
if (_optimizedExact)
{
int i = path.length();
while (i >= 0)
{
MappedResource<E> candidate = _exactMap.getBest(path, 0, i--);
if (candidate == null)
continue;

matchedPath = candidate.getPathSpec().matched(path);
if (matchedPath != null)
{
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
}
}
MappedResource<E> exact = _exactMap.get(path);
if (exact != null)
return exact.getPreMatched();
// If we reached here, there's NO optimized EXACT Match possible, skip simple match below
skipRestOfGroup = true;
}
Expand All @@ -196,17 +286,14 @@ public MatchedResource<E> getMatched(String path)
{
if (_optimizedPrefix)
{
int i = path.length();
while (i >= 0)
MappedResource<E> candidate = _prefixMap.getBest(path);
if (candidate != null)
{
MappedResource<E> candidate = _prefixMap.getBest(path, 0, i--);
if (candidate == null)
continue;

matchedPath = candidate.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
}

// If we reached here, there's NO optimized PREFIX Match possible, skip simple match below
skipRestOfGroup = true;
}
Expand Down Expand Up @@ -317,6 +404,7 @@ public boolean put(PathSpec pathSpec, E resource)
// Note: Example exact in Regex that can cause problems `^/a\Q/b\E/` (which is only ever matching `/a/b/`)
// Note: UriTemplate can handle exact easily enough
_optimizedExact = false;
_orderIsSignificant = true;
}
break;
case PREFIX_GLOB:
Expand All @@ -333,6 +421,7 @@ public boolean put(PathSpec pathSpec, E resource)
// Note: Example Prefix in Regex that can cause problems `^/a/b+` or `^/a/bb*` ('b' one or more times)
// Note: Example Prefix in UriTemplate that might cause problems `/a/{b}/{c}`
_optimizedPrefix = false;
_orderIsSignificant = true;
}
break;
case SUFFIX_GLOB:
Expand All @@ -349,6 +438,29 @@ public boolean put(PathSpec pathSpec, E resource)
// Note: Example suffix in Regex that can cause problems `^.*/path/name.ext` or `^/a/.*(ending)`
// Note: Example suffix in UriTemplate that can cause problems `/{a}/name.ext`
_optimizedSuffix = false;
_orderIsSignificant = true;
}
break;
case ROOT:
if (pathSpec instanceof ServletPathSpec)
{
if (_servletRoot == null)
_servletRoot = entry;
}
else
{
_orderIsSignificant = true;
}
break;
case DEFAULT:
if (pathSpec instanceof ServletPathSpec)
{
if (_servletDefault == null)
_servletDefault = entry;
}
else
{
_orderIsSignificant = true;
}
break;
default:
Expand Down Expand Up @@ -387,6 +499,7 @@ public boolean remove(PathSpec pathSpec)
_exactMap.remove(exact);
// Recalculate _optimizeExact
_optimizedExact = canBeOptimized(PathSpecGroup.EXACT);
_orderIsSignificant = nonServletPathSpec();
}
break;
case PREFIX_GLOB:
Expand All @@ -396,6 +509,7 @@ public boolean remove(PathSpec pathSpec)
_prefixMap.remove(prefix);
// Recalculate _optimizePrefix
_optimizedPrefix = canBeOptimized(PathSpecGroup.PREFIX_GLOB);
_orderIsSignificant = nonServletPathSpec();
}
break;
case SUFFIX_GLOB:
Expand All @@ -405,8 +519,23 @@ public boolean remove(PathSpec pathSpec)
_suffixMap.remove(suffix);
// Recalculate _optimizeSuffix
_optimizedSuffix = canBeOptimized(PathSpecGroup.SUFFIX_GLOB);
_orderIsSignificant = nonServletPathSpec();
}
break;
case ROOT:
_servletRoot = _mappings.stream()
.filter(mapping -> mapping.getPathSpec().getGroup() == PathSpecGroup.ROOT)
.filter(mapping -> mapping.getPathSpec() instanceof ServletPathSpec)
.findFirst().orElse(null);
_orderIsSignificant = nonServletPathSpec();
break;
case DEFAULT:
_servletDefault = _mappings.stream()
.filter(mapping -> mapping.getPathSpec().getGroup() == PathSpecGroup.DEFAULT)
.filter(mapping -> mapping.getPathSpec() instanceof ServletPathSpec)
.findFirst().orElse(null);
_orderIsSignificant = nonServletPathSpec();
break;
}
}

Expand All @@ -420,6 +549,12 @@ private boolean canBeOptimized(PathSpecGroup suffixGlob)
.allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec);
}

private boolean nonServletPathSpec()
{
return _mappings.stream()
.allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec);
}

@Override
public String toString()
{
Expand Down

0 comments on commit 4916377

Please sign in to comment.