sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1510889 - /sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
Date Tue, 06 Aug 2013 09:07:16 GMT
Author: desruisseaux
Date: Tue Aug  6 09:07:16 2013
New Revision: 1510889

URL: http://svn.apache.org/r1510889
Log:
Reduce the scope of the 'synchronized (loader)' block, in order to reduce contention in highly
multi-thread environment.

Modified:
    sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java

Modified: sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java?rev=1510889&r1=1510888&r2=1510889&view=diff
==============================================================================
--- sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
[UTF-8] (original)
+++ sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
[UTF-8] Tue Aug  6 09:07:16 2013
@@ -96,47 +96,55 @@ final class DataStoreRegistry {
         List<DataStoreProvider> deferred = null;
         try {
             /*
-             * TODO: we may have thread contention here.
-             * We are waiting to see if this is a problem in practice.
+             * All usages of 'loader' and its 'providers' iterator must be protected in a
synchronized block,
+             * because ServiceLoader is not thread-safe. We try to keep the synhronization
block as small as
+             * possible for less contention. In particular, the canOpen(connector) method
call may be costly.
              */
+            final Iterator<DataStoreProvider> providers;
+            DataStoreProvider candidate;
             synchronized (loader) {
-search:         for (final DataStoreProvider candidate : loader) {
-                    switch (candidate.canOpen(connector)) {
-                        /*
-                         * Stop at the first provider claiming to be able to read the storage.
-                         * We will exit the synchronized block before to open the storage.
-                         */
-                        case SUPPORTED: {
-                            provider = candidate;
-                            deferred = null;
-                            break search;
-                        }
-                        /*
-                         * If a provider doesn't have enough bytes for answering the question,
-                         * try again after this loop with more bytes in the buffer, unless
we
-                         * found an other provider.
-                         */
-                        case INSUFFICIENT_BYTES: {
-                            if (deferred == null) {
-                                deferred = new LinkedList<>();
-                            }
-                            deferred.add(candidate);
-                            break;
-                        }
-                        /*
-                         * If a provider doesn't know whether it can open the given storage,
-                         * we will try it only if we find no provider retuning SUPPORTED.
-                         *
-                         * TODO: What to do if we find more than one provider here? We can
not invoke
-                         *       provider.open(connector) in a try … catch block because
it may leave
-                         *       the StorageConnector in an invalid state in case of failure.
-                         */
-                        case UNDETERMINED: {
-                            provider = candidate;
-                            break;
+                providers = loader.iterator();
+                candidate = providers.hasNext() ? providers.next() : null;
+            }
+search:     while (candidate != null) {
+                switch (candidate.canOpen(connector)) {
+                    /*
+                     * Stop at the first provider claiming to be able to read the storage.
+                     * Do not iterate over the list of deferred providers (if any).
+                     */
+                    case SUPPORTED: {
+                        provider = candidate;
+                        deferred = null;
+                        break search;
+                    }
+                    /*
+                     * If a provider doesn't have enough bytes for answering the question,
+                     * try again after this loop with more bytes in the buffer, unless we
+                     * found an other provider.
+                     */
+                    case INSUFFICIENT_BYTES: {
+                        if (deferred == null) {
+                            deferred = new LinkedList<>();
                         }
+                        deferred.add(candidate);
+                        break;
+                    }
+                    /*
+                     * If a provider doesn't know whether it can open the given storage,
+                     * we will try it only if we find no provider retuning SUPPORTED.
+                     *
+                     * TODO: What to do if we find more than one provider here? We can not
invoke
+                     *       provider.open(connector) in a try … catch block because it
may leave
+                     *       the StorageConnector in an invalid state in case of failure.
+                     */
+                    case UNDETERMINED: {
+                        provider = candidate;
+                        break;
                     }
                 }
+                synchronized (loader) {
+                    candidate = providers.hasNext() ? providers.next() : null;
+                }
             }
             /*
              * If any provider did not had enough bytes for answering the 'canOpen(…)'
question,
@@ -146,7 +154,7 @@ search:         for (final DataStoreProv
             if (deferred != null) {
 search:         while (!deferred.isEmpty() && connector.prefetch()) {
                     for (final Iterator<DataStoreProvider> it=deferred.iterator();
it.hasNext();) {
-                        final DataStoreProvider candidate = it.next();
+                        candidate = it.next();
                         switch (candidate.canOpen(connector)) {
                             case SUPPORTED:          provider = candidate; break search;
                             case UNDETERMINED:       provider = candidate; break;



Mime
View raw message