Skip to content

Commit

Permalink
[performance] LocalFile: delete Files of Directories in parallel #1461
Browse files Browse the repository at this point in the history
There is no honest progress report as before. Just fixed the
InfiniteProgress to be thread safe and honestly using it.

#1461
  • Loading branch information
EcljpseB0T authored and jukzi committed Sep 27, 2024
1 parent 31da653 commit 6d514be
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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<Future<IStatus>> 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<Path> ds = Files.newDirectoryStream(target.toPath())) {
ds.forEach(p -> {
Future<IStatus> 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<IStatus> 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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<IFileStore> 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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 6d514be

Please sign in to comment.