aherbert commented on PR #276: URL: https://github.com/apache/commons-collections/pull/276#issuecomment-1492497122
> I think that we can make a change work for AbstractHashedMap. This is another variant of having to second guess what a user may have overridden. If we ignore anything outside of Collections (i.e. user code that has extended classes) then we have to support what we have. Some classes extend AbstractHashedMap and override `put` (e.g. AbstractReferenceMap); some override `createEntry` (AbstractLinkedMap). AbstractLinkedMap even overrides `createEntry` where the `convertKey` method is called the second time. There does not seem to be an easy point at which to insert new code that avoids a duplicate conversion. > Improving the performance of implementations that do not override the put() method. Performance improvement is negligible for Collections as no map overrides `convertKey` except CaseInsensitveMap. So changing a lot of the library to support unknown downstream users may be fixing a non-issue. > There is a possible implementation using a ThreadLocal variable IIUC ThreadLocal is not relevant for this as the map is not thread safe for put anyway. Any cache implementation that does not call convertKey and reuses an existing conversion would have to be robust to clearing its own cache. Otherwise you can get a stale converted key for a mutable key object. Currently I would favour the documentation of how to implement a class that perform convertKey using a cache. The user can then be made aware of downsides to possible implementations and can choose the best method for their use case. -- 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]
