showuon commented on a change in pull request #11367:
URL: https://github.com/apache/kafka/pull/11367#discussion_r720672004
##########
File path:
streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java
##########
@@ -29,21 +29,33 @@
import java.util.Iterator;
import java.util.List;
import java.util.NavigableMap;
-import java.util.Set;
+import java.util.NavigableSet;
import java.util.TreeMap;
import java.util.TreeSet;
+import java.util.concurrent.ConcurrentSkipListMap;
public class InMemoryKeyValueStore implements KeyValueStore<Bytes, byte[]> {
private static final Logger LOG =
LoggerFactory.getLogger(InMemoryKeyValueStore.class);
private final String name;
- private final NavigableMap<Bytes, byte[]> map = new TreeMap<>();
+ private final boolean copyOnRange;
+ private final NavigableMap<Bytes, byte[]> map;
+
private volatile boolean open = false;
private long size = 0L; // SkipListMap#size is O(N) so we just do our best
to track it
+ // for tests-only
public InMemoryKeyValueStore(final String name) {
+ this(name, true);
+ }
+
+ public InMemoryKeyValueStore(final String name, final boolean copyOnRange)
{
this.name = name;
+ this.copyOnRange = copyOnRange;
+ // if we do not need to copy on range, then we should use concurrent
skiplist map
+ // to avoid concurrent modificiation exception
+ this.map = copyOnRange ? new TreeMap<>() : new
ConcurrentSkipListMap<>();
Review comment:
@guozhangwang , I saw you added the `ConcurrentSkipListMap` back for not
copy-on-range case, but at the same time, we still have `synchronized` lock for
map operation methods. That seems over-locked, and should have worse
performance. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
