Perfomance and Other Improvements

Question

I am trying to wrap my arms around all the digital files we have, so I thought I would organize all of our pictures and videos into folders named after dates. I'm learning F#, and this felt like a good chance for me to exercise that muscle.

Please review the code. I'm open to any criticisms, but I also have some specific areas of interest:

1. I feel like the filtering out of the thumbs.db file should be inside the sequence expression. What's a good way to do that?

2. In getNewFilename, that function and its internal incrementFilename function each have an expression checking for the existence of the file. I think that check could be cleanly put into a single check, but I can't think quite how.

3. It took 1 hour and 10 minutes to copy 120GB in over 28,000 files. This is on a quad-core laptop with a 7,200 RPM drive. Realizing that the drive is the limiting factor for this work, and there could be clashes in filenames, what might be some good concurrent processing techniques to possibly have this code finish sooner?

open System
open System.Drawing
open System.IO
open System.Text
open System.Text.RegularExpressions

let rec getAllFiles baseDir =
    seq {
        yield! Directory.EnumerateFiles(baseDir)

        for dir in Directory.EnumerateDirectories(baseDir) do
            yield! getAllFiles dir
    }


let dateTakenFromExif file =
    let r = new Regex(":")
    use fs = new FileStream(file, FileMode.Open, FileAccess.Read)
    use myImage = Image.FromStream(fs, false, false)
    let propItem = myImage.GetPropertyItem(36867)
    let dateTaken = r.Replace(Encoding.UTF8.GetString(propItem.Value), "-", 2)
    DateTime.Parse(dateTaken)

let getDateTaken file =
    try
        dateTakenFromExif file
    with
    | :? Exception ->
        File.GetLastWriteTime(file)   //Use the last write time in the event that the file was moved/copied

let addToFile file n =
  match n with
  | 2 ->
    let fileDir = Path.GetDirectoryName(file)
    let nextFile = Path.GetFileNameWithoutExtension(file) + "-" + n.ToString() + Path.GetExtension(file)
    Path.Combine(fileDir, nextFile)
  | _ -> 
    let prev = n-1
    file.Replace("-" + prev.ToString(), "-" + n.ToString())

let getNewFilename newFilePath =
    let rec incrementFilename file n =
        let filenameIncremented = addToFile file n
        match File.Exists(filenameIncremented) with
        | false -> filenameIncremented
        | true -> incrementFilename filenameIncremented (n+1)

    match File.Exists(newFilePath) with
    | false -> newFilePath
    | true  -> incrementFilename newFilePath 2

let move destinationRoot files =
    let moveHelper file =
        let dateTaken = getDateTaken file
        let finalPath = Path.Combine(destinationRoot, dateTaken.Year.ToString(), dateTaken.ToString("yyyy-MM-dd"))
        if not(Directory.Exists(finalPath)) then Directory.CreateDirectory(finalPath) |> ignore

        let newFile = getNewFilename (Path.Combine(finalPath, Path.GetFileName(file)))

        try
          File.Copy(file, newFile)
        with
        | :? Exception as e ->
          failwith (sprintf "error renaming %s to %s\n%s" file newFile e.Message)

    files |> Seq.iter moveHelper


let moveFrom source =
  getAllFiles source
    |> Seq.filter (fun f -> Path.GetExtension(f).ToLower() <> ".db")  //exlcude the thumbs.db files
    |> move """C:\_EXTERNAL_DRIVE\_Camera"""
  printfn "Done"


#time
moveFrom """C:\Users\Mike\Pictures\To Network"""
moveFrom """C:\_EXTERNAL_DRIVE\Camera"""

Answer 1

A few things:

There's already a recursive overload of EnumerateFiles so you don't need getAllFiles. Your top level function would be:

let moveFrom source =
  Directory.EnumerateFiles(source, "*", SearchOption.AllDirectories)
    |> Seq.filter (fun f -> Path.GetExtension(f).ToLower() <> ".db")  //exlcude the thumbs.db files
    |> move """C:\_EXTERNAL_DRIVE\_Camera"""
  printfn "Done"

:? Exception is a type test pattern match, which is unnecessary in a try/with because with only fires for exceptions anyway. You could do this:

let getDateTaken file =
  try dateTakenFromExif file
  with _ -> File.GetLastWriteTime(file)

and this:

try File.Copy(file, newFile)
with e -> failwith (sprintf "error renaming %s to %s\n%s" file newFile e.Message)

instead.

addToFile could be merged into getNewFileName to form a simpler function without repetition:

let getNewFilename newFilePath =
  let rec loop file n =
    if File.Exists(file) then 
      let fileDir = Path.GetDirectoryName(file)
      let nextFile = Path.GetFileNameWithoutExtension(file) + "-" + n.ToString() + Path.GetExtension(file)
      loop (Path.Combine(fileDir, nextFile)) (n+1)
    else file
  loop newFilePath 2

For easy parallelization you might try using PSeq ("parallel sequence") from the F# PowerPack. You might see some gains from simply doing this:

files |> PSeq.iter moveHelper