aherbert commented on PR #276:
URL:
https://github.com/apache/commons-collections/pull/276#issuecomment-1489489428
On further thought, caching the key conversion is not always going to work.
The conversion must be done for each new call to the map with a key as it may
be mutable.
This test works on the current implementation. It fails when the conversion
is cached as the StringBuilder object is the same, even though it now is a
different converted key:
```
@Test
public void testCache() {
CaseInsensitiveMap<Object, Integer> map = new CaseInsensitiveMap<>();
Assertions.assertNull(map.get("foo"));
StringBuilder sb = new StringBuilder();
map.put(sb, 1);
sb.append("foo");
Assertions.assertNull(map.get(sb)); // fails with a cached conversion as
the map returns 1
}
```
Note that adding mutable items, changing them, and adding again is not a
typical use case. But for the CaseInsensitiveMap it can be as all keys are
stored converted.
So this leads us back to adding more methods to reuse the key converted in
`put` when creating a new entry. Wherever this change is introduced, it will
potentially be a functionally breaking change as methods that could have been
overridden may no longer be used. So the minimum breaking change would be to
make the change in `CaseInsensitiveMap`. Only an external library that extends
this class, and overrides `put`, `addMapping` and/or `createEntry` will be
affected. Adding new methods to `AbstractHashMap` to replace those which
duplicate key conversion may reveal more issues when this path is followed. For
example note that `AbstractLinkedMap` also calls `convertKey` in an override of
`createEntry`, but it doesn't override `put`. So this would be affected.
Currently I am not convinced that this performance enhancement can be
cleanly integrated with the current code. It would be easier for an end user to
just extend the map, cache key conversion and not deliberately break it with
mutable keys. This should be done if `put` is the largest part of the
performance overhead of using the map.
--
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]
