JavaHamcrest
JavaHamcrest copied to clipboard
hamcrest
hasKey performance issue
Why IsMapContaining.hasKey(K key) is iterating over whole map, instead of calling Map.containsKey? Latter is much faster!
Same problem with IsIterableContaining.hasItem(T item). Instead of wrapping the element in an equalTo() matcher, it could check if the iterable is a Collection and use the Collection.contains() method, or use a separate IsCollectionContaining matcher that only accepts Collection types and then also performs the test using the Collection.contains() method.
I just had a look at implementation of IsMapContaining.hasKey(K key). I get the impression that the current implementation is the way it is because of simplicity and consistency. That method has the same structure as every other method in the IsMapContaining class. This means there is only a single implementation of the matchesSafely method.
It would certainly be possible to create an optimised matcher especially for the case of matching an expected literal key inside a map, or a literal item inside a collection. It would be a slight amount of extra complexity for the better performance.
Out of interest, is the performance of this matcher causing any problems for your code?
Not as of yet, I have to admit.
However, I have written a library that relies heavily on Hamcrest that validates method preconditions and I can foresee that some methods accepting maps or collections may be inside heavily used loops.
Normally, such an algorithm would operate in linear time when using hash maps or hash sets, but with the current implementation of these matchers such an algorithm would perform in O(n * m) time, where n is the number of collections to process and m is the number of elements in these collections.
@Nibsi , it looks like I'm bitten with the same issue. Have you had a chance to optimize org.hamcrest.collection.IsMapContaining#matchesSafely ?
@tumbarumba @vlsi sorry guys, I completely got sidetracked. Thanks for the reminder. I'm busy over the weekend, but I'll definitely hop to it the coming week. Stay tuned.
I know this issue is old, but I went ahead and prepared a Pull Request that implements this proposed optimization. @tumbarumba and @nibsi, would you mind looking at this change if you have some time, please?
