DonalEvans commented on a change in pull request #7519:
URL: https://github.com/apache/geode/pull/7519#discussion_r839805711
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableByteArrayList.java
##########
@@ -102,22 +103,13 @@ public int lastIndexOf(Object o) {
}
/**
- * @param remove in order (smallest to largest) list of indexes to remove
+ * @param indexes list of indexes to remove
*/
- public void removeIndexes(List<Integer> remove) {
- int removeIndex = 0;
- int firstIndexToRemove = remove.get(0);
- ListIterator<byte[]> iterator = listIterator(firstIndexToRemove);
-
- // Iterates only through the indexes to remove
- for (int i = firstIndexToRemove; i <= remove.get(remove.size() - 1); i++) {
- byte[] element = iterator.next();
- if (i == remove.get(removeIndex)) {
- iterator.remove();
- memberOverhead -= calculateByteArrayOverhead(element);
- removeIndex++;
- }
+ public void removeIndexes(List<Integer> indexes) {
+ if(indexes.size() > 1) {
+ indexes.sort(Comparator.reverseOrder());
}
+ indexes.forEach(index -> this.remove((int)index));
Review comment:
This change causes a significant performance hit, since we're using a
LinkedList implementation that makes removing an element at an index O(N).
Previously, we iterated the list once and removed the indexes as we found them,
which is O(N); with this change, we iterate the list once for each index we
want to remove, which is O(M*N).
--
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]
