dschneider-pivotal commented on a change in pull request #7418:
URL: https://github.com/apache/geode/pull/7418#discussion_r828321437
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##########
@@ -161,6 +167,26 @@ public int llen() {
return elementList.size();
}
+ /**
+ * @param region the region this instance is stored in
+ * @param key the name of the list to add to
+ * @return the element actually popped
+ */
+ public byte[] rpop(Region<RedisKey, RedisData> region, RedisKey key) {
+ byte newVersion;
+ int index = elementList.size() - 1;
+ byte[] popped;
+ RemoveElementsByIndex removed;
+ synchronized (this) {
+ newVersion = incrementAndGetVersion();
+ popped = removeLastElement();
+ removed = new RemoveElementsByIndex(newVersion);
Review comment:
the creation of RemoveElementsByIndex should be outside the sync block
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##########
@@ -161,6 +167,26 @@ public int llen() {
return elementList.size();
}
+ /**
+ * @param region the region this instance is stored in
+ * @param key the name of the list to add to
+ * @return the element actually popped
+ */
+ public byte[] rpop(Region<RedisKey, RedisData> region, RedisKey key) {
Review comment:
I think you can refactor rpop and lpop to both call a common private
method that takes the index of the element to remove. For lpop that index would
be 0 and for rpop that index would be size-1. The common method can use the
existing remove(int) method to modify the SizableByteArrayList so you will not
need to add new method to it. I looked at the implementation of LinkedList and
using remove(int) on it has roughly the same performance as removeFirst() and
removeLast() and the code will be simpler and more consistent (the primary and
secondary will do the remove in the same way)
--
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]
