sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 01/03: Do more work in background thread.
Date Mon, 04 May 2020 16:05:14 GMT
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

commit 32a44159bc0b8556b261fc47f042b6c002eac872
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Mon May 4 15:06:08 2020 +0200

    Do more work in background thread.
---
 .../java/org/apache/sis/gui/coverage/GridView.java |  3 +-
 .../apache/sis/gui/dataset/ResourceExplorer.java   | 66 ++++++++++++++++-
 .../org/apache/sis/gui/dataset/ResourceTree.java   | 83 ++++++++++++----------
 .../apache/sis/internal/gui/BackgroundThreads.java | 46 ++++++++++--
 4 files changed, 152 insertions(+), 46 deletions(-)

diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
index 3493479..ec1031d 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
@@ -118,7 +118,7 @@ public class GridView extends Control {
      * since "real" caching is done by {@link org.apache.sis.image.ComputedImage}. The purpose
of this cache is
      * to remember that a tile is immediately available and that we do not need to start
a background thread.
      */
-    private final GridTileCache tiles = new GridTileCache();
+    private final GridTileCache tiles;
 
     /**
      * The most recently used tile. Cached separately because it will be the desired tile
in the vast majority
@@ -233,6 +233,7 @@ public class GridView extends Control {
         headerFormat     = NumberFormat.getIntegerInstance();
         cellFormat       = new CellFormat(this);
         statusBar        = new StatusBar(referenceSystems);
+        tiles            = new GridTileCache();
         tileWidth        = 1;
         tileHeight       = 1;       // For avoiding division by zero.
 
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java
index da80440..55050f2 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java
@@ -22,6 +22,7 @@ import javafx.beans.property.ReadOnlyProperty;
 import javafx.beans.property.ReadOnlyObjectWrapper;
 import javafx.collections.ListChangeListener;
 import javafx.collections.ObservableList;
+import javafx.concurrent.Task;
 import javafx.geometry.Orientation;
 import javafx.scene.Node;
 import javafx.scene.layout.Region;
@@ -41,6 +42,8 @@ import org.apache.sis.storage.GridCoverageResource;
 import org.apache.sis.util.collection.TableColumn;
 import org.apache.sis.util.resources.Vocabulary;
 import org.apache.sis.internal.gui.Resources;
+import org.apache.sis.internal.gui.BackgroundThreads;
+import org.apache.sis.internal.gui.ExceptionReporter;
 
 
 /**
@@ -123,6 +126,12 @@ public class ResourceExplorer extends WindowManager {
     private double dividerPosition;
 
     /**
+     * Used for building {@link #viewTab} and {@link #tableTab} when first needed.
+     * This is {@code null} the rest of the time.
+     */
+    private transient DataTabBuilder builder;
+
+    /**
      * Creates a new panel for exploring resources.
      */
     public ResourceExplorer() {
@@ -179,6 +188,49 @@ public class ResourceExplorer extends WindowManager {
     }
 
     /**
+     * A background task for building the content of {@link #viewTab} and {@link #tableTab}
when first needed.
+     * This task does not load data, it is only for building the GUI. This operation is longer
for those tabs
+     * when built for the first time.
+     */
+    private final class DataTabBuilder extends Task<CoverageExplorer> {
+        /**
+         * The resource to show after construction is completed, or {@code null} if none.
+         */
+        volatile Resource resource;
+
+        /**
+         * Creates a new data tabs builder. The given resource will be shown after
+         * the tabs are ready, unless {@link #resource} is modified after construction.
+         */
+        DataTabBuilder(final Resource resource) {
+            this.resource = resource;
+        }
+
+        /** Builds the tabs GUI components in a background thread. */
+        @Override protected CoverageExplorer call() {
+            return new CoverageExplorer();
+        }
+
+        /** Shows the resource after the tabs GUI are built. */
+        @Override protected void succeeded() {
+            builder  = null;
+            coverage = getValue();
+            updateDataTab(resource);
+        }
+
+        /** Invoked if the tabs can not be built. */
+        @Override protected void failed() {
+            builder = null;
+            ExceptionReporter.show(this);
+        }
+
+        /** Should never happen, but defined as a safety. */
+        @Override protected void cancelled() {
+            builder = null;
+        }
+    }
+
+    /**
      * Returns resources for current locale.
      */
     @Override
@@ -260,16 +312,26 @@ public class ResourceExplorer extends WindowManager {
      * @param  resource  the resource to set, or {@code null} if none.
      */
     private void updateDataTab(final Resource resource) {
+        /*
+         * If tabs are being built in a background thread, wait for construction to finish.
+         * The builder will callback this `updateDataTab(resource)` method when ready.
+         */
+        if (builder != null) {
+            builder.resource = resource;
+            return;
+        }
         Region       image = null;
         Region       table = null;
         FeatureSet   data  = null;
         ImageRequest grid  = null;
         CoverageExplorer.View type = null;
         if (resource instanceof GridCoverageResource) {
-            grid = new ImageRequest((GridCoverageResource) resource, null, 0);
             if (coverage == null) {
-                coverage = new CoverageExplorer();      // TODO: build in background thread.
+                builder = new DataTabBuilder(resource);
+                BackgroundThreads.execute(builder);
+                return;
             }
+            grid  = new ImageRequest((GridCoverageResource) resource, null, 0);
             image = coverage.getDataView(CoverageExplorer.View.IMAGE);
             table = coverage.getDataView(CoverageExplorer.View.TABLE);
             type  = viewTab.isSelected() ? CoverageExplorer.View.IMAGE : CoverageExplorer.View.TABLE;
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
index a32df07..e5ef83a 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
@@ -549,10 +549,7 @@ public class ResourceTree extends TreeView<Resource> {
                 isChildrenKnown = true;                 // Set first for avoiding to repeat
in case of failure.
                 final Resource resource = getValue();
                 if (resource instanceof Aggregate) {
-                    final GetChildren task = new GetChildren((Aggregate) resource);
-                    task.setOnSucceeded((event) -> setResources(((GetChildren) event.getSource()).getValue()));
-                    task.setOnFailed((event) -> setResources(((GetChildren) event.getSource()).unloadable()));
-                    BackgroundThreads.execute(task);
+                    BackgroundThreads.execute(new GetChildren((Aggregate) resource));
                     children.add(new Item(PseudoResource.LOADING));     // Temporary node
with "loading" text.
                 }
             }
@@ -560,50 +557,58 @@ public class ResourceTree extends TreeView<Resource> {
         }
 
         /**
-         * Sets the resources after the background task completed.
-         * This method must be invoked in the JavaFX thread.
+         * The task to execute in a background thread for fetching the children.
          */
-        private void setResources(final List<TreeItem<Resource>> result) {
-            super.getChildren().setAll(result);
-        }
-    }
+        private final class GetChildren extends Task<List<TreeItem<Resource>>>
{
+            /**
+             * The aggregate from which to get the children.
+             */
+            private final Aggregate resource;
 
-    /**
-     * The task to execute in a background thread for fetching the children.
-     *
-     * @todo Draw a progress bar.
-     */
-    private static final class GetChildren extends Task<List<TreeItem<Resource>>>
{
-        /**
-         * The aggregate from which to get the children.
-         */
-        private final Aggregate resource;
+            /**
+             * Creates a new background task for fetching the children from the given resource.
+             */
+            GetChildren(final Aggregate resource) {
+                this.resource = resource;
+            }
 
-        /**
-         * Creates a new background task for fetching the children from the given resource.
-         */
-        GetChildren(final Aggregate resource) {
-            this.resource = resource;
-        }
+            /**
+             * Invoked in a background thread for fetching the children of the resource
+             * specified at construction time.
+             */
+            @Override
+            protected List<TreeItem<Resource>> call() throws DataStoreException
{
+                final List<TreeItem<Resource>> items = new ArrayList<>();
+                for (final Resource component : resource.components()) {
+                    items.add(new Item(component));
+                }
+                return items;
+            }
 
-        /**
-         * Invoked in a background thread for fetching the children of the resource specified
-         * at construction time.
-         */
-        @Override
-        protected List<TreeItem<Resource>> call() throws DataStoreException {
-            final List<TreeItem<Resource>> items = new ArrayList<>();
-            for (final Resource component : resource.components()) {
-                items.add(new Item(component));
+            /**
+             * Invoked in JavaFX thread if children have been loaded successfully.
+             */
+            @Override
+            protected void succeeded() {
+                setResources(getValue());
+            }
+
+            /**
+             * Invoked in JavaFX thread if children can not be loaded.
+             * This method set a placeholder items with error message.
+             */
+            @Override
+            protected void failed() {
+                setResources(Collections.singletonList(new Item(new Unloadable(getException()))));
             }
-            return items;
         }
 
         /**
-         * Returns an item to set instead of the result when the operation failed.
+         * Sets the resources after the background task completed.
+         * This method must be invoked in the JavaFX thread.
          */
-        final List<TreeItem<Resource>> unloadable() {
-            return Collections.singletonList(new Item(new Unloadable(getException())));
+        private void setResources(final List<TreeItem<Resource>> result) {
+            super.getChildren().setAll(result);
         }
     }
 
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
index 473d4d2..3023c36 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
@@ -21,7 +21,9 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
+import javafx.application.Platform;
 import org.apache.sis.internal.system.Modules;
 import org.apache.sis.internal.system.Threads;
 import org.apache.sis.util.logging.Logging;
@@ -73,13 +75,36 @@ public final class BackgroundThreads extends AtomicInteger implements
ThreadFact
     }
 
     /**
-     * Executes the given task in a background thread.
-     * This method can be invoked from any thread.
+     * Executes the given task in a background thread. This method can be invoked from any
thread.
+     * If the current thread is not the {@linkplain Platform#isFxApplicationThread() JavaFX
thread},
+     * then this method assumes that current thread is already a background thread and execute
the
+     * given task in that thread.
      *
      * @param  task  the task to execute.
      */
     public static void execute(final Runnable task) {
-        EXECUTOR.execute(task);
+        if (Platform.isFxApplicationThread()) {
+            EXECUTOR.execute(task);
+        } else {
+            task.run();
+            /*
+             * The given task is almost always a `javafx.concurrent.Task` which needs to
do some work
+             * in JavaFX thread after the background thread finished. Because this method
is normally
+             * invoked from JavaFX thread, the caller does not expect its JavaFX properties
to change
+             * concurrently. For avoiding unexpected behavior, we wait for the given task
to complete
+             * the work that it may be doing in JavaFX thread. We rely on the fact that `task.run()`
+             * has already done its `Platform.runLater(…)` calls at this point, and the
call that we
+             * are doing below is guaranteed to be executed after the calls done by `task.run()`.
+             * The timeout is low because the tasks on JavaFX threads are supposed to be
very short.
+             */
+            final CountDownLatch c = new CountDownLatch(1);
+            Platform.runLater(c::countDown);
+            try {
+                c.await(5, TimeUnit.SECONDS);
+            } catch (InterruptedException e) {
+                interrupted("execute", e);
+            }
+        }
     }
 
     /**
@@ -98,8 +123,21 @@ public final class BackgroundThreads extends AtomicInteger implements
ThreadFact
              * Someone does not want to wait for termination.
              * Closes the data stores now even if some of them may still be in use.
              */
-            Logging.recoverableException(Logging.getLogger(Modules.APPLICATION), BackgroundThreads.class,
"stop", e);
+            interrupted("stop", e);
         }
         ResourceLoader.closeAll();
     }
+
+    /**
+     * Invoked when waiting for an operation to complete and the wait has been interrupted.
+     * It should not happen, but if it happens anyway we just log a warning and continue.
+     * Note that this is not very different than continuing after the timeout elapsed.
+     * In both cases the risk is that some data structure may be in inconsistent state.
+     *
+     * @param  method  the method which has been interrupted.
+     * @param  e       the exception that interrupted the waiting process.
+     */
+    private static void interrupted(final String method, final InterruptedException e) {
+        Logging.unexpectedException(Logging.getLogger(Modules.APPLICATION), BackgroundThreads.class,
method, e);
+    }
 }


Mime
View raw message