From 6d514be2aca4b9be131184b63570fd4351c47c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Fri, 20 Sep 2024 16:21:43 +0200 Subject: [PATCH] [performance] LocalFile: delete Files of Directories in parallel #1461 There is no honest progress report as before. Just fixed the InfiniteProgress to be thread safe and honestly using it. https://github.com/eclipse-platform/eclipse.platform/issues/1461 --- .../filesystem/local/InfiniteProgress.java | 61 ++++++------- .../internal/filesystem/local/LocalFile.java | 85 ++++++++++++++----- .../core/tests/filesystem/FileStoreTest.java | 45 ++++++++++ .../tests/resources/ResourceTestUtil.java | 9 +- 4 files changed, 138 insertions(+), 62 deletions(-) diff --git a/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/InfiniteProgress.java b/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/InfiniteProgress.java index 719acde7d83..b052c1b1051 100644 --- a/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/InfiniteProgress.java +++ b/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/InfiniteProgress.java @@ -14,56 +14,47 @@ package org.eclipse.core.internal.filesystem.local; import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.ProgressMonitorWrapper; /** * This class provides a simulation of progress. This is useful * for situations where computing the amount of work to do in advance * is too costly. The monitor will accept any number of calls to - * {@link #worked(int)}, and will scale the actual reported work appropriately + * {@link #worked()}, and will scale the actual reported work appropriately * so that the progress never quite completes. */ -public class InfiniteProgress extends ProgressMonitorWrapper { - /* - * Fields for progress monitoring algorithm. - * Initially, give progress for every 4 resources, double - * this value at halfway point, then reset halfway point - * to be half of remaining work. (this gives an infinite - * series that converges at total work after an infinite - * number of resources). - */ - private int totalWork; - private int currentIncrement = 4; - private int halfWay; - private int nextProgress = currentIncrement; - private int worked = 0; +public class InfiniteProgress { + private final int MAX_TICKS = 172; // will be reached after ~ 1 Billion #worked() + private int worked; + private int nextTickAfter = 4; + private int workReported; + private final IProgressMonitor monitor; protected InfiniteProgress(IProgressMonitor monitor) { - super(monitor); + this.monitor = monitor; } - @Override - public void beginTask(String name, int work) { - super.beginTask(name, work); - this.totalWork = work; - this.halfWay = totalWork / 2; + public void beginTask(String name) { + monitor.beginTask(name, MAX_TICKS); } - @Override - public void worked(int work) { - if (--nextProgress <= 0) { - //we have exhausted the current increment, so report progress - super.worked(1); - worked++; - if (worked >= halfWay) { - //we have passed the current halfway point, so double the - //increment and reset the halfway point. - currentIncrement *= 2; - halfWay += (totalWork - halfWay) / 2; + public synchronized void subTask(String name) { + monitor.subTask(name); + } + + public synchronized void worked() { + worked += 1; + if (worked > nextTickAfter) { + worked = 0; + // starting with linear progress converging to asymptotic logarithmic progress: + nextTickAfter = 1 + (int) (nextTickAfter * 1.1f); + if (workReported < MAX_TICKS) { + workReported++; + monitor.worked(1); } - //reset the progress counter to another full increment - nextProgress = currentIncrement; } } + public boolean isCanceled() { + return monitor.isCanceled(); + } } diff --git a/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java b/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java index 49d876876d0..0870195db76 100644 --- a/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java +++ b/resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java @@ -29,11 +29,21 @@ import java.nio.file.AccessDeniedException; import java.nio.file.CopyOption; import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.NoSuchFileException; +import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.ForkJoinWorkerThread; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.eclipse.core.filesystem.EFS; import org.eclipse.core.filesystem.IFileInfo; import org.eclipse.core.filesystem.IFileStore; @@ -50,6 +60,7 @@ import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.MultiStatus; import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.core.runtime.OperationCanceledException; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.SubMonitor; import org.eclipse.osgi.util.NLS; @@ -181,14 +192,16 @@ public static final void transferAttributes(IFileInfo sourceInfo, IFileStore des @Override public void delete(int options, IProgressMonitor monitor) throws CoreException { - if (monitor == null) + if (monitor == null) { monitor = new NullProgressMonitor(); - else - monitor = new InfiniteProgress(monitor); + } try { - IStatus result = internalDelete(file, monitor); - if (!result.isOK()) + InfiniteProgress infMonitor = new InfiniteProgress(monitor); + infMonitor.beginTask(NLS.bind(Messages.deleting, file)); + IStatus result = internalDelete(file, infMonitor, FILE_SERVICE); + if (!result.isOK()) { throw new CoreException(result); + } } finally { monitor.done(); } @@ -261,40 +274,69 @@ public int hashCode() { return file.hashCode(); } + private static final ForkJoinPool FILE_SERVICE = createExecutor(Math.max(1, Runtime.getRuntime().availableProcessors())); + + private static ForkJoinPool createExecutor(int threadCount) { + return new ForkJoinPool(threadCount, pool -> { + final ForkJoinWorkerThread worker = ForkJoinPool.defaultForkJoinWorkerThreadFactory.newThread(pool); + worker.setName("LocalFile Deleter"); //$NON-NLS-1$ + worker.setDaemon(true); + return worker; + }, // + /* UncaughtExceptionHandler */ null, // + /* asyncMode */ false, // Last-In-First-Out is important to delete child before parent folders + /* corePoolSize */ 0, // + /* maximumPoolSize */ threadCount, // + /* minimumRunnable */ 0, null, // delete algorithm does not need any + /* keepAliveTime */ 1, TimeUnit.MINUTES); // pool terminates 1 thread per + } + /** - * Deletes the given file recursively, adding failure info to - * the provided status object. The filePath is passed as a parameter - * to optimize java.io.File object creation. - */ - private IStatus internalDelete(File target, IProgressMonitor monitor) { - SubMonitor subMonitor = SubMonitor.convert(monitor, NLS.bind(Messages.deleting, target), IProgressMonitor.UNKNOWN); - subMonitor.checkCanceled(); + * Deletes the given file recursively, adding failure info to + * the provided status object. The filePath is passed as a parameter + * to optimize java.io.File object creation. + */ + private IStatus internalDelete(File target, InfiniteProgress infMonitor, ExecutorService executorService) { + infMonitor.subTask(NLS.bind(Messages.deleting, target)); + if (infMonitor.isCanceled()) { + throw new OperationCanceledException(); + } + List> futures = new ArrayList<>(); try { try { // First try to delete - this should succeed for files and symbolic links to directories. Files.deleteIfExists(target.toPath()); + infMonitor.worked(); return Status.OK_STATUS; } catch (AccessDeniedException e) { // If the file is read only, it can't be deleted via Files.deleteIfExists() // see https://bugs.eclipse.org/bugs/show_bug.cgi?id=500306 if (target.delete()) { + infMonitor.worked(); return Status.OK_STATUS; } throw e; } - } catch (DirectoryNotEmptyException e) { + } catch (DirectoryNotEmptyException dne) { // The target is a directory and it's not empty - File[] directoryElements = target.listFiles(); - if (directoryElements == null) { - directoryElements = new File[0]; // Unlikely to happen if the directory is not empty - } - subMonitor.setWorkRemaining(directoryElements.length); // Try to delete all children. - MultiStatus deleteChildrenStatus = new MultiStatus(Policy.PI_FILE_SYSTEM, EFS.ERROR_DELETE, Messages.deleteProblem, null); - for (File element : directoryElements) { - deleteChildrenStatus.add(internalDelete(element, subMonitor.split(1))); + try (DirectoryStream ds = Files.newDirectoryStream(target.toPath())) { + ds.forEach(p -> { + Future future = executorService.submit(() -> internalDelete(p.toFile(), infMonitor, executorService)); + futures.add(future); + }); + } catch (IOException streamException) { + return createErrorStatus(target, Messages.deleteProblem, streamException); } + MultiStatus deleteChildrenStatus = new MultiStatus(Policy.PI_FILE_SYSTEM, EFS.ERROR_DELETE, Messages.deleteProblem, null); + for (Future f : futures) { + try { + deleteChildrenStatus.add(f.get()); + } catch (InterruptedException | ExecutionException ee) { + deleteChildrenStatus.add(createErrorStatus(target, Messages.deleteProblem, ee)); + } + } // Abort if one of the children couldn't be deleted. if (!deleteChildrenStatus.isOK()) { return deleteChildrenStatus; @@ -303,6 +345,7 @@ private IStatus internalDelete(File target, IProgressMonitor monitor) { // Try to delete the root directory try { if (Files.deleteIfExists(target.toPath())) { + infMonitor.worked(); return Status.OK_STATUS; } } catch (Exception e1) { diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/FileStoreTest.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/FileStoreTest.java index c34f843f6da..e37e349224c 100755 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/FileStoreTest.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/FileStoreTest.java @@ -428,6 +428,51 @@ public void testGetStat() throws CoreException, IOException { assertTrue(EFS.NONE != stat); } + @Test + public void testDelete() throws Throwable { + IFileStore tempC = createDir(randomUniqueNotExistingFileStore(), true); + System.out.println("Threadcount=" + Runtime.getRuntime().availableProcessors()); + { + IFileStore singleDirectory = tempC.getChild("tree"); + createDir(singleDirectory, true); + List treeItems = new ArrayList<>(); + for (int i = 0; i < 2300; i++) { + treeItems.add(singleDirectory.getChild("file" + i)); + } + + createTree(treeItems.toArray(IFileStore[]::new)); + assertTrue(singleDirectory.fetchInfo().exists()); + long n0 = System.nanoTime(); + singleDirectory.delete(EFS.NONE, null); + long n1 = System.nanoTime(); + System.out.println("delete singleDirectory took ms:" + (n1 - n0) / 1_000_000); + assertFalse(singleDirectory.fetchInfo().exists()); + } + + { + IFileStore binaryTree = tempC.getChild("tree"); + createDir(binaryTree, true); + createChildDirs(binaryTree, 2, 10); + assertTrue(binaryTree.fetchInfo().exists()); + long n0 = System.nanoTime(); + binaryTree.delete(EFS.NONE, null); + long n1 = System.nanoTime(); + System.out.println("delete binaryTree took ms:" + (n1 - n0) / 1_000_000); + assertFalse(binaryTree.fetchInfo().exists()); + } + } + + private void createChildDirs(IFileStore parent, int count, int depth) throws CoreException { + if (depth <= 0) { + return; + } + for (int i = 0; i < count; i++) { + IFileStore child = parent.getChild("dir-" + depth + "-" + i); + child.mkdir(EFS.NONE, null); + createChildDirs(child, count, depth - 1); + } + } + @Test public void testMove() throws Throwable { /* build scenario */ diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/ResourceTestUtil.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/ResourceTestUtil.java index 7e61c74065d..3a4f99e722c 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/ResourceTestUtil.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/ResourceTestUtil.java @@ -18,7 +18,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -126,11 +125,9 @@ public static void createInFileSystem(IFileStore file) throws CoreException, IOE */ public static void createInFileSystem(IFileStore file, int fileSizeInBytes) throws CoreException, IOException { file.getParent().mkdir(EFS.NONE, null); - try (OutputStream output = new BufferedOutputStream(file.openOutputStream(EFS.NONE, null))) { - for (int size = 0; size < fileSizeInBytes; size++) { - output.write(RANDOM.nextInt(Byte.SIZE)); - } - } + byte[] content = new byte[fileSizeInBytes]; + RANDOM.nextBytes(content); + file.write(content, EFS.NONE, null); } /**