tutorials icon indicating copy to clipboard operation
tutorials copied to clipboard

Lock striping Java code might be flawed

Open Marcono1234 opened this issue 3 years ago • 0 comments

It looks the Java code in the article https://www.baeldung.com/java-lock-stripping (respectively this directory of the repository) might be flawed:

  • The name of the parameter int threads is misleading. The code uses CompletableFuture.supplyAsync(Supplier) which uses the common pool with a fixed number of threads. The parameter threads is rather a task count divided by 4.
  • The usage of Striped with HashMap does not appear to be thread-safe. HashMap is in general not thread-safe; trying to lock only certain areas of it with a striped lock can most likely not be thread-safe because even if two entries were placed in separate buckets, the map also has global state such as the total map size which has to be accessed in a thread-safe way. Besides that, it is unlikely that the current usage of the striped lock really matches how HashMap distributes the entries, especially when the map is rehashed when the capacity is increased during addition of new entries.
  • The comparison is done with a ConcurrentHashMap with additional synchronization. The experiment and benchmarking code seems to apply the locks to ConcurrentHashMap as well. This usage is rather weird and probably not close to reality because it is unlikely that one puts additional synchronization around ConcurrentHashMap usage. Instead it might be more interesting to see comparison between a striped lock implementation and direct ConcurrentHashMap usage (without any additional synchronization on it).

The idea of showcasing how Guava's Striped can be used and comparing it with another concurrent collection is really great, but for that HashMap should probably not be used due to the issues outlined above.

Marcono1234 avatar Apr 19 '22 19:04 Marcono1234