commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bojan Vukojevic (JIRA)" <j...@apache.org>
Subject [jira] Commented: (TRANSACTION-26) applyDeletes in commons-transaction-1.2 is very slow in case when large number of files needs to be deleted from the directory
Date Thu, 06 Sep 2007 16:18:31 GMT

    [ 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.


Mime
View raw message