Remove a volatile read/write in SmallSortedMap.entrySet()

I don't know why this was volatile.

Volatile semantics are not strong enough for making this read-then-write a
transaction. You'd need a mutex or AtomicReference to achieve that.

Maybe it's so that if someone else has already made a EntrySet on another
thread, we can reuse that? But, it's probably cheaper to make a new EntrySet
probably than to do the volatile read-or-write (this is just a guess, may be wrong, I'm
not super experienced with the cost of volatiles).

The EntrySet is stateless (except for the state of its containing
SmallSortedMap) so it's fine to have more than one of them.

The commit introducing this doesn't explain why it's volatile, but the method
comment referring to "Similar to the AbstractMap implementation of {@code
keySet()}" may have some clues.

AbstractMap.keySet uses a transient keySet, not a volatile keySet.

Is it possible that this initial implementation mistook transient for volatile?

Anyway, it's unnecessary, let's rip it out and enjoy a little more performance.

PiperOrigin-RevId: 662071950
pull/17786/head
Mark Hansen 6 months ago committed by Copybara-Service
parent 744c9ddfc5
commit 4e6b93ca4e
  1. 2
      java/core/src/main/java/com/google/protobuf/SmallSortedMap.java

@ -106,7 +106,7 @@ class SmallSortedMap<K extends Comparable<K>, V> extends AbstractMap<K, V> {
private boolean isImmutable;
// The EntrySet is a stateless view of the Map. It's initialized the first
// time it is requested and reused henceforth.
private volatile EntrySet lazyEntrySet;
private EntrySet lazyEntrySet;
private Map<K, V> overflowEntriesDescending;
private SmallSortedMap() {

Loading…
Cancel
Save