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 <[email protected]> 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); + } }
