This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 908f46ccbe Reduce the risk of exhaustive scan of the EPSG database
when searching for the authority code of a singleton.
908f46ccbe is described below
commit 908f46ccbe04806a21081a65329427523cb2d38e
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Sat Aug 30 10:58:40 2025 +0200
Reduce the risk of exhaustive scan of the EPSG database when searching for
the authority code of a singleton.
---
.../apache/sis/referencing/AuthorityFactories.java | 4 +-
.../factory/ConcurrentAuthorityFactory.java | 122 +++++++++++++++++----
.../factory/IdentifiedObjectFinder.java | 17 ++-
.../referencing/factory/sql/EPSGCodeFinder.java | 2 +-
4 files changed, 113 insertions(+), 32 deletions(-)
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/AuthorityFactories.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/AuthorityFactories.java
index 287bd3fadb..83fd38a632 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/AuthorityFactories.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/AuthorityFactories.java
@@ -260,7 +260,7 @@ final class AuthorityFactories<T extends AuthorityFactory>
extends LazySet<T> {
delegate(fallback(e).newIdentifiedObjectFinder());
}
- /** Lookups objects which are approximately equal, using the
fallback if necessary. */
+ /** Looks up objects which are approximately equal, using the
fallback if necessary. */
@Override public Set<IdentifiedObject> find(final IdentifiedObject
object) throws FactoryException {
for (;;) try { // Executed at most twice.
return super.find(object);
@@ -269,7 +269,7 @@ final class AuthorityFactories<T extends AuthorityFactory>
extends LazySet<T> {
}
}
- /** Lookups an object which is approximately equal, using the
fallback if necessary. */
+ /** Looks up an object which is approximately equal, using the
fallback if necessary. */
@Override public IdentifiedObject findSingleton(final
IdentifiedObject object) throws FactoryException {
for (;;) try { // Executed at most twice.
return super.findSingleton(object);
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java
index af6b17c7c0..62f0b9b3f7 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java
@@ -1850,6 +1850,13 @@ public abstract class ConcurrentAuthorityFactory<DAO
extends GeodeticAuthorityFa
super(factory);
}
+ /**
+ * Returns the factory given at construction time.
+ */
+ private ConcurrentAuthorityFactory<?> factory() {
+ return (ConcurrentAuthorityFactory<?>) factory;
+ }
+
/**
* Acquires a new {@linkplain #finder}.
* The {@link #release()} method must be invoked in a {@code finally}
block after the call to {@code acquire}.
@@ -1868,7 +1875,7 @@ public abstract class ConcurrentAuthorityFactory<DAO
extends GeodeticAuthorityFa
assert Thread.holdsLock(this);
assert (acquireCount == 0) == (finder == null) : acquireCount;
if (acquireCount == 0) {
- final GeodeticAuthorityFactory delegate =
((ConcurrentAuthorityFactory<?>) factory).getDataAccess();
+ final GeodeticAuthorityFactory delegate =
factory().getDataAccess();
/*
* Set `acquireCount` only after we succeed in fetching the
factory, and before any operation on it.
* The intent is to get ConcurrentAuthorityFactory.release()
invoked if and only if the getDataAccess()
@@ -1893,7 +1900,7 @@ public abstract class ConcurrentAuthorityFactory<DAO
extends GeodeticAuthorityFa
}
if (--acquireCount == 0) {
finder = null;
- ((ConcurrentAuthorityFactory<?>) factory).release(null, null,
null);
+ factory().release(null, null, null);
}
}
@@ -1911,36 +1918,79 @@ public abstract class ConcurrentAuthorityFactory<DAO
extends GeodeticAuthorityFa
}
}
+ /**
+ * Returns the index in the cached {@code Set<IdentifiedObject>[]}
array for a result using the given finder.
+ * The index depends on the finder configuration. The argument should
be either {@link #finder} if non-null,
+ * or {@code this} otherwise.
+ */
+ private static int index(final IdentifiedObjectFinder finder) {
+ int index = finder.getSearchDomain().ordinal();
+ if (finder.isIgnoringAxes()) index += DOMAIN_COUNT;
+ return index;
+ }
+
/**
* Returns the cached value for the given object, or {@code null} if
none.
+ * This is checked by {@link #find(IdentifiedObject)} before actual
search.
* The returned set (if non-null) is unmodifiable.
+ *
+ * @param object the user-specified object to search.
+ * @return the cached result of the find operation, or {@code null} if
none.
*/
@Override
final Set<IdentifiedObject> getFromCache(final IdentifiedObject
object) {
- final Map<IdentifiedObject, Set<IdentifiedObject>[]> findPool =
((ConcurrentAuthorityFactory<?>) factory).findPool;
+ // `finder` should never be null since this method is not invoked
directly by this Finder.
+ return getFromCache(object, index(finder));
+ }
+
+ /**
+ * Implementation of {@link #getFromCache(IdentifiedObject)} with the
specified index to use in the cache.
+ * The index depends on the finder configuration.
+ *
+ * @param object the user-specified object to search.
+ * @param index value of {@link #index(IdentifiedObjectFinder)} or
custom slot.
+ * @return the cached result of the find operation, or {@code null} if
none.
+ */
+ private Set<IdentifiedObject> getFromCache(final IdentifiedObject
object, final int index) {
+ final Map<IdentifiedObject, Set<IdentifiedObject>[]> findPool =
factory().findPool;
synchronized (findPool) {
final Set<IdentifiedObject>[] entry = findPool.get(object);
if (entry != null) {
- // `finder` may be null if this method is invoked directly
by this Finder.
- return entry[index(finder != null ? finder : this)];
+ return entry[index];
}
}
return null;
}
/**
- * Stores the given result in the cache. This method wraps or copies
the given set
- * in an unmodifiable set and returns the result.
+ * Stores the given result in the cache. This method copies the given
set in a new unmodifiable
+ * set and returns the result. The copy is needed because, with the
current implementation of
+ * <abbr>EPSG</abbr> factory, {@code result} may be a lazy set with a
connection to the database.
+ *
+ * @param object the user-specified object which was searched.
+ * @param result the search result. It will be copied.
+ * @return a set with the same content as {@code result}.
*/
@Override
- final Set<IdentifiedObject> cache(final IdentifiedObject object,
Set<IdentifiedObject> result) {
- final Map<IdentifiedObject, Set<IdentifiedObject>[]> findPool =
((ConcurrentAuthorityFactory<?>) factory).findPool;
- result = CollectionsExt.unmodifiableOrCopy(result);
+ final Set<IdentifiedObject> cache(final IdentifiedObject object, final
Set<IdentifiedObject> result) {
+ // `finder` should never be null since this method is not invoked
directly by this Finder.
+ return cache(object, CollectionsExt.copyPreserveOrder(result),
index(finder));
+ }
+
+ /**
+ * Implementation of {@link #cache(IdentifiedObject, Set)} with the
specified index to use in the cache.
+ * The index depends on the finder configuration.
+ *
+ * @param object the user-specified object which was searched.
+ * @param result the search result. The copy, if needed, shall be
done by the caller.
+ * @param index value of {@link #index(IdentifiedObjectFinder)} or
custom slot.
+ * @return a set with the same content as {@code result}.
+ */
+ private Set<IdentifiedObject> cache(final IdentifiedObject object,
Set<IdentifiedObject> result, final int index) {
+ final Map<IdentifiedObject, Set<IdentifiedObject>[]> findPool =
factory().findPool;
synchronized (findPool) {
- // `finder` should never be null since this method is not
invoked directly by this Finder.
- final int i = index(finder);
final Set<IdentifiedObject>[] entry =
findPool.computeIfAbsent(object, Finder::createCacheEntry);
- final Set<IdentifiedObject> existing = entry[i];
+ final Set<IdentifiedObject> existing = entry[index];
if (existing != null) {
return existing;
}
@@ -1950,36 +2000,60 @@ public abstract class ConcurrentAuthorityFactory<DAO
extends GeodeticAuthorityFa
break;
}
}
- entry[i] = result;
+ entry[index] = result;
}
return result;
}
/**
* Creates an initially empty cache entry for the given object.
+ * Used in lambda expression and defined as a separated method because
of generic type.
+ * The {@code object} argument is present only for having the required
method signature.
+ *
+ * @param object the user-specified object which was searched.
+ * @return a new array to use as a cache for the specified object.
*/
@SuppressWarnings({"unchecked", "rawtypes"}) // Generic
array creation.
private static Set<IdentifiedObject>[]
createCacheEntry(IdentifiedObject object) {
- return new Set[DOMAIN_COUNT * 2];
+ return new Set[DOMAIN_COUNT * 3];
}
/**
- * Returns the index in the cached {@code Set<IdentifiedObject>[]}
array
- * for a result using the given finder.
+ * Looks up an object from this authority factory which is
approximately equal to the specified object.
+ * The default implementation performs the same lookup as the Data
Access Object and caches the result.
+ *
+ * @param object the object looked up.
+ * @return the identified object, or {@code null} if none or ambiguous.
*/
- private static int index(final IdentifiedObjectFinder finder) {
- int i = finder.getSearchDomain().ordinal();
- if (finder.isIgnoringAxes()) i += DOMAIN_COUNT;
- return i;
+ @Override
+ public IdentifiedObject findSingleton(final IdentifiedObject object)
throws FactoryException {
+ final int index = index(this) + 2*DOMAIN_COUNT;
+ Set<IdentifiedObject> result = getFromCache(object, index);
+ if (result == null) {
+ synchronized (this) {
+ try {
+ acquire();
+ result =
CollectionsExt.singletonOrEmpty(finder.findSingleton(object));
+ } finally {
+ release();
+ }
+ }
+ cache(object, result, index);
+ }
+ factory().findPoolLatestQueries.markAsUsed(object);
+ return CollectionsExt.first(result);
}
/**
- * Looks up an object from this authority factory which is
approximately equal to the specified object.
+ * Looks up objects from this authority factory which are
approximately equal to the specified object.
* The default implementation performs the same lookup as the Data
Access Object and caches the result.
+ *
+ * @param object the object looked up.
+ * @return the identified objects, or an empty set if not found.
*/
@Override
public Set<IdentifiedObject> find(final IdentifiedObject object)
throws FactoryException {
- Set<IdentifiedObject> candidate = getFromCache(object);
+ Set<IdentifiedObject> candidate = getFromCache(object,
index(this));
if (candidate == null) {
/*
* Nothing has been found in the cache. Delegates the search
to the Data Access Object.
@@ -2001,7 +2075,7 @@ public abstract class ConcurrentAuthorityFactory<DAO
extends GeodeticAuthorityFa
* We keep a strong reference only for the top-level object, not
for the intermediate searches,
* because strong references to intermediate objects already exist
in the top-level object.
*/
- ((ConcurrentAuthorityFactory<?>)
factory).findPoolLatestQueries.markAsUsed(object);
+ factory().findPoolLatestQueries.markAsUsed(object);
return candidate;
}
}
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java
index 6d950d6db6..6284d56dae 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java
@@ -36,6 +36,7 @@ import org.apache.sis.referencing.privy.LazySet;
import org.apache.sis.util.ComparisonMode;
import org.apache.sis.util.Disposable;
import org.apache.sis.util.Utilities;
+import org.apache.sis.util.OptionalCandidate;
import org.apache.sis.util.privy.Constants;
import org.apache.sis.util.collection.BackingStoreException;
import org.apache.sis.util.logging.Logging;
@@ -304,7 +305,11 @@ public class IdentifiedObjectFinder {
/**
* Returns the cached value for the given object, or {@code null} if none.
+ * This is checked by {@link #find(IdentifiedObject)} before actual search.
* The returned set (if non-null) should be unmodifiable.
+ *
+ * @param object the user-specified object to search.
+ * @return the cached result of the find operation, or {@code null} if
none.
*/
Set<IdentifiedObject> getFromCache(final IdentifiedObject object) {
return (wrapper != null) ? wrapper.getFromCache(object) : null;
@@ -314,7 +319,8 @@ public class IdentifiedObjectFinder {
* Stores the given result in the cache, if any. If this method chooses to
cache the given set,
* then it shall wrap or copy the given set in an unmodifiable set and
returns the result.
*
- * @param result the search result as a modifiable set.
+ * @param object the user-specified object which was searched.
+ * @param result the search result. It will potentially be copied.
* @return a set with the same content as {@code result}.
*/
Set<IdentifiedObject> cache(final IdentifiedObject object,
Set<IdentifiedObject> result) {
@@ -325,7 +331,7 @@ public class IdentifiedObjectFinder {
}
/**
- * Lookups only one object which is approximately equal to the specified
object.
+ * Looks up only one object which is approximately equal to the specified
object.
* This method invokes {@link #find(IdentifiedObject)}, then examine the
returned {@code Set} as below:
*
* <ul>
@@ -341,6 +347,7 @@ public class IdentifiedObjectFinder {
* @return the identified object, or {@code null} if none or ambiguous.
* @throws FactoryException if an error occurred while fetching the
authority code candidates.
*/
+ @OptionalCandidate
public IdentifiedObject findSingleton(final IdentifiedObject object)
throws FactoryException {
/*
* Do not invoke Set.size() because it may be a costly operation if
the subclass
@@ -368,7 +375,7 @@ public class IdentifiedObjectFinder {
}
/**
- * Lookups objects which are approximately equal to the specified object.
+ * Looks up objects which are approximately equal to the specified object.
* This method tries to instantiate objects identified by the {@linkplain
#getCodeCandidates set of candidate codes}
* using the {@linkplain #factory authority factory} specified at
construction time.
* {@link FactoryException}s thrown during object creations are logged and
otherwise ignored.
@@ -785,7 +792,7 @@ public class IdentifiedObjectFinder {
}
/**
- * Lookups objects which are approximately equal to the specified
object.
+ * Looks up objects which are approximately equal to the specified
object.
* The default method implementation delegates the work to the finder
specified by {@link #delegate()}.
*/
@Override
@@ -796,7 +803,7 @@ public class IdentifiedObjectFinder {
}
/**
- * Lookups only one object which is approximately equal to the
specified object.
+ * Looks up only one object which is approximately equal to the
specified object.
* The default method implementation delegates the work to the finder
specified by {@link #delegate()}.
*/
@Override
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/sql/EPSGCodeFinder.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/sql/EPSGCodeFinder.java
index d7a3d64524..ebe9fe21ef 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/sql/EPSGCodeFinder.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/factory/sql/EPSGCodeFinder.java
@@ -108,7 +108,7 @@ final class EPSGCodeFinder extends IdentifiedObjectFinder {
}
/**
- * Lookups objects which are approximately equal to the specified object.
+ * Looks up objects which are approximately equal to the specified object.
* This method temporarily disables warnings about deprecated objects.
*/
@Override
