samarthjain commented on a change in pull request #12109:
URL: https://github.com/apache/druid/pull/12109#discussion_r821035172
##########
File path:
processing/src/main/java/org/apache/druid/segment/DimensionDictionary.java
##########
@@ -47,102 +47,146 @@
private final Object2IntMap<T> valueToId = new Object2IntOpenHashMap<>();
private final List<T> idToValue = new ArrayList<>();
- private final ReentrantReadWriteLock lock;
+ private final StampedLock lock;
public DimensionDictionary()
{
- this.lock = new ReentrantReadWriteLock();
+ this.lock = new StampedLock();
Review comment:
I think it is worthwhile comparing benchmark runs using synchronized vs
stamped locks vs read write locks. The modern JVMs have significantly improved
performance for locks afforded by synchronized construct. Looking at this code,
it doesn't look like there is a lot of long running activity going around when
holding the write lock. So I suspect just using basic synchronized based
locking should suffice. One other behavior change with StampedLock is that it
is not reentrant unlike the `ReadWriteReentrantLock` and synchronized. That
makes the code slightly more difficult to maintain, but I personally am OK with
it if perf turns out to be better.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
