Thread safety of buffers and caches #38
Conversation
| return this.size() > configuration.getCapacityMultiObjects(); | ||
| } | ||
| }; | ||
| Collections.synchronizedMap( |
There was a problem hiding this comment.
Do we want to consider a ConcurrentHashMap so we don't lock on the entire map?
There was a problem hiding this comment.
Yes, ConcurrentHashMap is faster but does not provide the LRU eviction functionalities. Ilya is trying out with https://github.com/google/guava/wiki/CachesExplained. If that works, we can use the Guava Cache
There was a problem hiding this comment.
We probably don't want to take a dependency on Guava though :/ you run into dependency conflict issues in the hadoop stack, that's why we copied pasted Preconditions from Guava
There was a problem hiding this comment.
We could possibly shade Guava possibly. but let's evaluate if we really need it and the discuss with Oleg
| @@ -25,7 +27,8 @@ public AutoClosingCircularBuffer(int maxCapacity) { | |||
|
|
|||
| this.oldestIndex = 0; | |||
| this.capacity = maxCapacity; | |||
| this.buffer = new ArrayList<>(maxCapacity); | |||
| this.buffer = Collections.synchronizedList(new ArrayList<>(maxCapacity)); | |||
| // this.buffer = new ArrayList<>(maxCapacity); | |||
| @@ -4,14 +4,16 @@ | |||
| import java.io.Closeable; | |||
| import java.io.IOException; | |||
| import java.util.ArrayList; | |||
| import java.util.Collections; | |||
| import java.util.List; | |||
| import java.util.stream.Stream; | |||
|
|
|||
| /** | |||
| * A circular buffer of fixed capacity. Closes its elements before removing them. Not thread-safe. | |||
There was a problem hiding this comment.
"Not thread-safe" --> Does it still stand after this change?
Issue #, if available:
Description of changes:
This change wraps the LinkedHashMap and ArrayList used to access IOBlocks in Synchronized data structures to make operations thread safe.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.