Optimize IdentityMostlySingleton #3558
Conversation
| state = State.ANY; | ||
| // Use ArrayList, but make sure to use reference comparisons. | ||
| set = new ArrayList<>(); |
There was a problem hiding this comment.
I am partial to the version in #3550 because the variable named set is a set, and because it's shorter -- it doesn't need to re-implement logic for add and contains and therefore I find it easier to read and understand. As a minor benefit, #3550 type-checks under the Nullness Checker without the need for assumptions about use of the set variable in other subclasses of AbstractMostlySingleton (namely, @MonotonicNonNull on that variable, which is necessary in this pull request).
| state = State.ANY; | ||
| // Use ArrayList, but make sure to use reference comparisons. |
There was a problem hiding this comment.
In the other pull request, you said that you think using an array list is more efficient. Can you explain more why you think it is more efficient? MostlySingleton uses a HashSet, should we change that one to make it more efficient, too?
When I was debugging this code, I was very surprised that set was in fact an ArrayList. I wondered why that choice was made. Given that using an ArrayList and using List#contains is not the correct behavior, it made more sense to me to use an identity hash set. This also matches the implementation match MostlySingleton.
mernst
changed the title
No description provided.