ppkarwasz commented on code in PR #3050:
URL: https://github.com/apache/logging-log4j2/pull/3050#discussion_r1795248638
##########
log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java:
##########
@@ -350,82 +351,59 @@ public UnmodifiableArrayBackedMap copyAndRemove(String
key) {
}
/**
- * Creates a new instance that contains the same entries as this map,
minus all
- * of the keys passed in the arguments.
- *
- * @param key
- * @param value
- * @return
+ * Creates a new instance where the entries of provided keys are removed.
*/
public UnmodifiableArrayBackedMap copyAndRemoveAll(Iterable<String>
keysToRemoveIterable) {
+
+ // Short-circuit if the map is empty
if (isEmpty()) {
- // shortcut: if this map is empty, the result will continue to be
empty
return EMPTY_MAP;
}
- // now we build a Set of keys to remove
- Set<String> keysToRemoveSet;
- if (keysToRemoveIterable instanceof Set) {
- // we already have a set, let's cast it and reuse it
- keysToRemoveSet = (Set<String>) keysToRemoveIterable;
- } else {
- // iterate through the keys and build a set
- keysToRemoveSet = new HashSet<>();
- for (String key : keysToRemoveIterable) {
- keysToRemoveSet.add(key);
- }
- }
+ // Collect distinct keys to remove
+ final Set<String> keysToRemove = keysToRemoveIterable instanceof Set
+ ? (Set<String>) keysToRemoveIterable
+ : StreamSupport.stream(keysToRemoveIterable.spliterator(),
false)
+ .collect(Collectors.toSet());
- int firstIndexToKeep = -1;
- int lastIndexToKeep = -1;
- int destinationIndex = 0;
- int numEntriesKept = 0;
- // build the new map
- UnmodifiableArrayBackedMap newMap = new
UnmodifiableArrayBackedMap(numEntries);
- for (int indexInCurrentMap = 0; indexInCurrentMap < numEntries;
indexInCurrentMap++) {
- // for each key in this map, check whether it's in the set we
built above
- Object key = backingArray[getArrayIndexForKey(indexInCurrentMap)];
- if (!keysToRemoveSet.contains(key)) {
- // this key should be kept
- if (firstIndexToKeep == -1) {
- firstIndexToKeep = indexInCurrentMap;
- }
- lastIndexToKeep = indexInCurrentMap;
- } else if (lastIndexToKeep > 0) {
- // we hit a remove, copy any keys that are known ready
- int numEntriesToCopy = lastIndexToKeep - firstIndexToKeep + 1;
- System.arraycopy(
- backingArray,
- getArrayIndexForKey(firstIndexToKeep),
- newMap.backingArray,
- getArrayIndexForKey(destinationIndex),
- numEntriesToCopy * 2);
- firstIndexToKeep = -1;
- lastIndexToKeep = -1;
- destinationIndex += numEntriesToCopy;
- numEntriesKept += numEntriesToCopy;
- }
- }
+ // Create the new map
+ final UnmodifiableArrayBackedMap oldMap = this;
+ final int oldMapEntryCount = oldMap.numEntries;
+ final UnmodifiableArrayBackedMap newMap = new
UnmodifiableArrayBackedMap(oldMapEntryCount);
- if (lastIndexToKeep > -1) {
- // at least one key still requires copying
- int numEntriesToCopy = lastIndexToKeep - firstIndexToKeep + 1;
- System.arraycopy(
- backingArray,
- getArrayIndexForKey(firstIndexToKeep),
- newMap.backingArray,
- getArrayIndexForKey(destinationIndex),
- numEntriesToCopy * 2);
- numEntriesKept += numEntriesToCopy;
+ // Short-circuit if there is nothing to remove
+ if (keysToRemove.isEmpty()) {
+ System.arraycopy(oldMap.backingArray, 0, newMap.backingArray, 0,
oldMapEntryCount * 2);
+ newMap.numEntries = oldMapEntryCount;
+ return this;
}
- if (numEntriesKept == 0) {
- return EMPTY_MAP;
+ // Iterate over old map entries
+ int newMapEntryIndex = 0;
+ for (int oldMapEntryIndex = 0; oldMapEntryIndex < oldMapEntryCount;
oldMapEntryIndex++) {
+ final int oldMapKeyIndex = getArrayIndexForKey(oldMapEntryIndex);
Review Comment:
Great job, thanks!
--
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: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
