[
https://issues.apache.org/jira/browse/TRANSACTION-26?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12525449
]
Bojan Vukojevic commented on TRANSACTION-26:
--------------------------------------------
THx for looking at it, but I think you are wrong. This is the code from the bug
you are referencing:
protected static void applyDeletes(File removeDir, File targetDir, File
rootDir) throws IOException {
if (removeDir.isDirectory() && targetDir.isDirectory()) {
File[] files = removeDir.listFiles();
for (int i = 0; i < files.length; i++) {
File removeFile = files[i];
File targetFile = new File(targetDir, removeFile.getName());
if (!removeFile.isDirectory()) {
if (targetFile.exists()) {
if (!targetFile.delete()) {
throw new IOException("Could not delete file " +
removeFile.getName()
+ " in directory targetDir");
}
} else if(!targetFile.isFile()){ //============ CHANGE to
delete dangling link=============
//this is likely a dangling link
targetFile.delete();
}
// indicate, this has been done
removeFile.delete();
} else {
applyDeletes(removeFile, targetFile, rootDir);
}
// delete empty target directories, except root dir
if (!targetDir.equals(rootDir) && targetDir.list().length == 0)
{
targetDir.delete();
}
}
}
}
===================================
The proposed change is to pull check for the empty directory out of the loop
i.e. delete all files that are scheduled to be deleted and then delete the
directory, like this:
protected static void applyDeletes(File removeDir, File targetDir, File
rootDir) throws IOException {
if (removeDir.isDirectory() && targetDir.isDirectory()) {
File[] files = removeDir.listFiles();
for (int i = 0; i < files.length; i++) {
File removeFile = files[i];
File targetFile = new File(targetDir, removeFile.getName());
if (!removeFile.isDirectory()) {
if (targetFile.exists()) {
if (!targetFile.delete()) {
throw new IOException("Could not delete file " +
removeFile.getName()
+ " in directory targetDir");
}
} else if(!targetFile.isFile()){ //============ CHANGE to
delete dangling link=============
//this is likely a dangling link
targetFile.delete();
}
// indicate, this has been done
removeFile.delete();
} else {
applyDeletes(removeFile, targetFile, rootDir);
}
//this is where the directory deletion was
}
//new location for deletion of empty directories
//the reason to put it here is to check if the directory is empty
only after all files from that directory were deleted
// delete empty target directories, except root dir
if (!targetDir.equals(rootDir) && targetDir.list().length == 0)
{
targetDir.delete();
}
}
}
I hope the code example above clears things up.....
I was also wondering if I can become regular contributor to this project...
Thx!
> applyDeletes in commons-transaction-1.2 is very slow in case when large
> number of files needs to be deleted from the directory
> ------------------------------------------------------------------------------------------------------------------------------
>
> Key: TRANSACTION-26
> URL: https://issues.apache.org/jira/browse/TRANSACTION-26
> Project: Commons Transaction
> Issue Type: Improvement
> Affects Versions: 1.2
> Environment: Linux/JDK1.5
> Reporter: Bojan Vukojevic
> Assignee: Oliver Zeigermann
> Priority: Minor
> Fix For: 1.2
>
> Attachments: apply_delete_patch_1.2.txt
>
>
> method applyDeletes in FileResourceManager lists the directory after every
> file delete. In case when there is a large number of files in the directory
> that needs to be deleted, this can be very slow. I had a case where directory
> had ~100,000.00 files. It took 5 hours for this method to complete. Simple
> improvement can be done where the check for the empty directory is moved
> outside of the loop that is deleting the files, i.e.:
> Original:
> protected static void applyDeletes(File removeDir, File targetDir, File
> rootDir) throws IOException {
> if (removeDir.isDirectory() && targetDir.isDirectory()) {
> File[] files = removeDir.listFiles();
> for (int i = 0; i < files.length; i++) {
> File removeFile = files[i];
> File targetFile = new File(targetDir, removeFile.getName());
> if (removeFile.isFile()) {
> if (targetFile.exists()) {
> if (!targetFile.delete()) {
> throw new IOException("Could not delete file " +
> removeFile.getName()
> + " in directory targetDir");
> }
> }
> // indicate, this has been done
> removeFile.delete();
> } else {
> applyDeletes(removeFile, targetFile, rootDir);
> }
> // delete empty target directories, except root dir
> if (!targetDir.equals(rootDir) && targetDir.list().length ==
> 0) {
> targetDir.delete();
> }
> }
> }
> }
> =======================
> Much Faster:
> protected static void applyDeletes(File removeDir, File targetDir, File
> rootDir) throws IOException {
> if (removeDir.isDirectory() && targetDir.isDirectory()) {
> File[] files = removeDir.listFiles();
> for (int i = 0; i < files.length; i++) {
> File removeFile = files[i];
> File targetFile = new File(targetDir, removeFile.getName());
> if (removeFile.isFile()) {
> if (targetFile.exists()) {
> if (!targetFile.delete()) {
> throw new IOException("Could not delete file " +
> removeFile.getName()
> + " in directory targetDir");
> }
> }
> // indicate, this has been done
> removeFile.delete();
> } else {
> applyDeletes(removeFile, targetFile, rootDir);
> }
> }
> // delete empty target directories, except root dir
> if (!targetDir.equals(rootDir) && targetDir.list().length == 0) {
> targetDir.delete();
> }
> }
> }
> Below is the patch generated based on the released 1.2 version:
> Index:
> //10.1.31.20/bvukojev/DS7.1_EclipseWorkspace/asf-commons-transaction-1.2/src/java/org/apache/commons/transaction/file/FileResourceManager.java
> ===================================================================
> ---
> //10.1.31.20/bvukojev/DS7.1_EclipseWorkspace/asf-commons-transaction-1.2/src/java/org/apache/commons/transaction/file/FileResourceManager.java
> (revision 573053)
> +++
> //10.1.31.20/bvukojev/DS7.1_EclipseWorkspace/asf-commons-transaction-1.2/src/java/org/apache/commons/transaction/file/FileResourceManager.java
> (working copy)
> @@ -162,12 +162,12 @@
> removeFile.delete();
> } else {
> applyDeletes(removeFile, targetFile, rootDir);
> - }
> - // delete empty target directories, except root dir
> - if (!targetDir.equals(rootDir) && targetDir.list().length ==
> 0) {
> - targetDir.delete();
> - }
> + }
> }
> + // delete empty target directories, except root dir
> + if (!targetDir.equals(rootDir) && targetDir.list().length == 0) {
> + targetDir.delete();
> + }
> }
> }
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
