From a10b619ed91b6bb970e4b33a1901d61c94e97ef4 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Sun, 11 Aug 2024 17:07:20 -0700 Subject: [PATCH] Remove SmallSortedMap.lazyDescendingEntrySet field caching I've noted the rationale for this in the comment, and in the attached bug. This should save about 4-8 bytes of memory (on the 64bit server JVM depending on if compressed object pointers are on) per object with extensions. And 4 bytes on Android. PiperOrigin-RevId: 661900014 --- .../main/java/com/google/protobuf/SmallSortedMap.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/SmallSortedMap.java b/java/core/src/main/java/com/google/protobuf/SmallSortedMap.java index b7f7ffe6ef..1d5cddd868 100644 --- a/java/core/src/main/java/com/google/protobuf/SmallSortedMap.java +++ b/java/core/src/main/java/com/google/protobuf/SmallSortedMap.java @@ -108,7 +108,6 @@ class SmallSortedMap, V> extends AbstractMap { // time it is requested and reused henceforth. private volatile EntrySet lazyEntrySet; private Map overflowEntriesDescending; - private volatile DescendingEntrySet lazyDescendingEntrySet; private SmallSortedMap() { this.overflowEntries = Collections.emptyMap(); @@ -338,10 +337,12 @@ class SmallSortedMap, V> extends AbstractMap { } Set> descendingEntrySet() { - if (lazyDescendingEntrySet == null) { - lazyDescendingEntrySet = new DescendingEntrySet(); - } - return lazyDescendingEntrySet; + // Optimisation note: Many java.util.Map implementations would, here, cache the return value in + // a field, to avoid allocations for future calls to this method. But for us, descending + // iteration is rare, SmallSortedMaps are very common, and the entry set is only useful for + // iteration, which allocates anyway. The extra memory cost of the field (4-8 bytes) isn't worth + // it. See b/357002010. + return new DescendingEntrySet(); } /** @throws UnsupportedOperationException if {@link #makeImmutable()} has has been called. */