2
\$\begingroup\$

I'm working on some teaching examples showing the perils of accessing the Dictionary class concurrently. In the code below each function is designed to do a word count across a number of files. (This isn't the best way of doing the count but it's there to make a point about Dictionary.)

The first example, using a Dictionary and no parallelism works fine.

The second example, using a Dictionary and Array.parallel can fail with an exception and also produces different results each time. This is deliberate and is there to make the point.

The final example uses ConcurrentDictionary and seems to me to work fine. My concern is: given this uses a delegate to do the count addition, is this version genuinely thread safe?

// Simple version:
let WordCount dirPath wildCard =

   let wordCounts = Dictionary<string, int>()

   let ProcessFile fileName =
      let text = File.ReadAllText(fileName)

      text.Split([|'.'; ' '; '\r'|], StringSplitOptions.RemoveEmptyEntries)
      |> Array.map (fun w -> w.Trim())
      |> Array.filter (fun w -> w.Length > 2)
      |> Array.iter (fun w -> 
         let ok, count = wordCounts.TryGetValue(w)
         if ok then
            wordCounts.[w] <- count+1
         else
            wordCounts.[w] <- 1)

   Directory.EnumerateFiles(dirPath, wildCard)
   |> Seq.iter ProcessFile

   wordCounts
   // |> Seq.sumBy (fun kv -> kv.Value)
   |> Seq.sortBy (fun kv -> -kv.Value)

// Naive concurrent version:
let WordCount2 dirPath wildCard =

   let wordCounts = Dictionary<string, int>()

   let ProcessFile fileName =
      let text = File.ReadAllText(fileName)

      text.Split([|'.'; ' '; '\r'|], StringSplitOptions.RemoveEmptyEntries)
      |> Array.map (fun w -> w.Trim())
      |> Array.filter (fun w -> w.Length > 2)
      |> Array.iter (fun w -> 
         let ok, count = wordCounts.TryGetValue(w)
         if ok then
            wordCounts.[w] <- count+1
         else
            wordCounts.[w] <- 1)

   Directory.EnumerateFiles(dirPath, wildCard)
   |> Array.ofSeq
   |> Array.Parallel.iter ProcessFile

   wordCounts   
   |> Seq.sumBy (fun kv -> kv.Value)
   // |> Seq.sortBy (fun kv -> -kv.Value)

// Safe concurrent version:
let WordCount3 dirPath wildCard =

   let wordCounts = ConcurrentDictionary<string, int>()

   let ProcessFile fileName =
      let text = File.ReadAllText(fileName)

      text.Split([|'.'; ' '; '\r'|], StringSplitOptions.RemoveEmptyEntries)
      |> Array.map (fun w -> w.Trim())
      |> Array.filter (fun w -> w.Length > 2)
      |> Array.iter (fun w -> wordCounts.AddOrUpdate(w, 1, (fun _ count -> count+1 )) |> ignore)

   Directory.EnumerateFiles(dirPath, wildCard)
   |> Array.ofSeq
   |> Array.Parallel.iter ProcessFile

   wordCounts 
   |> Seq.sumBy (fun kv -> kv.Value)
   // |> Seq.sortBy (fun kv -> -kv.Value)
\$\endgroup\$
  • \$\begingroup\$ Why wouldn't it be thread-safe? Isn't that what ConcurrentDictionary is for? \$\endgroup\$ – Daniel May 6 '14 at 14:05
  • 1
    \$\begingroup\$ ConcurrentDictionary provides thread-safety through atomic operations, so you shouldn't call ContainsKey followed by Item, you should call TryGetValue. That's why it has methods like GetOrAdd and AddOrUpdate that the regular Dictionary doesn't. \$\endgroup\$ – Daniel May 6 '14 at 14:31
  • \$\begingroup\$ @Daniel Yes, I know. All I'm saying is that it makes sense to ask “Am I using ConcurrentDictionary correctly?” Because you can use it incorrectly. \$\endgroup\$ – svick May 6 '14 at 14:38
3
\$\begingroup\$

If you just used your original code (TryGetValue followed by an indexer set) with ConcurrentDictionary, then that wouldn't be thread-safe.

But you're using the AddOrUpdate method instead, which is guaranteed to be thread-safe (i.e. atomic), so this use is thread-safe.


One other nitpick:

Directory.EnumerateFiles(dirPath, wildCard)
|> Array.ofSeq

If you don't actually want seq, but an array, Directory has a method just for that: Directory.GetFiles.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.