Skip to content

Commit

Permalink
Inline ArrayList's array into SmallSortedMap
Browse files Browse the repository at this point in the history 
Store them as Object[], because you can't have a Entry[], because Entry[] has a generic 'this' over <K, V>, you get error "generic array creation" if you try to make "new Entry[]".

And if you try to cast Object[] to Entry[], you get ClassCastException. So I've just made it an object array that we cast when reading out of.

This should save memory and improve performance, because we have fewer pointers to chase.

Also fixed some warnings about unnecessary unchecked suppressions, and type-name should end with T.

PiperOrigin-RevId: 658585983
  • Loading branch information
@mhansen @copybara-github
mhansen authored and copybara-github committed Aug 2, 2024
1 parent 58a97a4 commit 910f627
Showing 1 changed file with 69 additions and 43 deletions.
112 changes: 69 additions & 43 deletions java/core/src/main/java/com/google/protobuf/SmallSortedMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import java.util.AbstractMap;
import java.util.AbstractSet;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -64,25 +63,21 @@ class SmallSortedMap<K extends Comparable<K>, V> extends AbstractMap<K, V> {
* Creates a new instance for mapping FieldDescriptors to their values. The {@link
* #makeImmutable()} implementation will convert the List values of any repeated fields to
* unmodifiable lists.
*
* @param arraySize The size of the entry array containing the lexicographically smallest
* mappings.
*/
static <FieldDescriptorType extends FieldSet.FieldDescriptorLite<FieldDescriptorType>>
SmallSortedMap<FieldDescriptorType, Object> newFieldMap() {
return new SmallSortedMap<FieldDescriptorType, Object>() {
static <FieldDescriptorT extends FieldSet.FieldDescriptorLite<FieldDescriptorT>>
SmallSortedMap<FieldDescriptorT, Object> newFieldMap() {
return new SmallSortedMap<FieldDescriptorT, Object>() {
@Override
@SuppressWarnings("unchecked")
public void makeImmutable() {
if (!isImmutable()) {
for (int i = 0; i < getNumArrayEntries(); i++) {
final Map.Entry<FieldDescriptorType, Object> entry = getArrayEntryAt(i);
final Map.Entry<FieldDescriptorT, Object> entry = getArrayEntryAt(i);
if (entry.getKey().isRepeated()) {
final List<?> value = (List) entry.getValue();
entry.setValue(Collections.unmodifiableList(value));
}
}
for (Map.Entry<FieldDescriptorType, Object> entry : getOverflowEntries()) {
for (Map.Entry<FieldDescriptorT, Object> entry : getOverflowEntries()) {
if (entry.getKey().isRepeated()) {
final List<?> value = (List) entry.getValue();
entry.setValue(Collections.unmodifiableList(value));
Expand All @@ -99,10 +94,14 @@ static <K extends Comparable<K>, V> SmallSortedMap<K, V> newInstanceForTest() {
return new SmallSortedMap<>();
}

// The "entry array" is actually a List because generic arrays are not
// allowed. ArrayList also nicely handles the entry shifting on inserts and
// removes.
private List<Entry> entryList;
// Only has Entry elements inside.
// Can't declare this as Entry[] because Entry is generic, so you get "generic array creation"
// error. Instead, use an Object[], and cast to Entry on read.
// null Object[] means 'empty'.
private Object[] entries;
// Number of elements in entries that are valid, like ArrayList.size.
private int entriesSize;

private Map<K, V> overflowEntries;
private boolean isImmutable;
// The EntrySet is a stateless view of the Map. It's initialized the first
Expand All @@ -112,16 +111,15 @@ static <K extends Comparable<K>, V> SmallSortedMap<K, V> newInstanceForTest() {
private volatile DescendingEntrySet lazyDescendingEntrySet;

private SmallSortedMap() {
this.entryList = Collections.emptyList();
this.overflowEntries = Collections.emptyMap();
this.overflowEntriesDescending = Collections.emptyMap();
}

/** Make this map immutable from this point forward. */
public void makeImmutable() {
if (!isImmutable) {
// Note: There's no need to wrap the entryList in an unmodifiableList
// because none of the list's accessors are exposed. The iterator() of
// Note: There's no need to wrap the entries in an unmodifiableList
// because none of the array's accessors are exposed. The iterator() of
// overflowEntries, on the other hand, is exposed so it must be made
// unmodifiable.
overflowEntries =
Expand All @@ -143,12 +141,17 @@ public boolean isImmutable() {

/** @return The number of entries in the entry array. */
public int getNumArrayEntries() {
return entryList.size();
return entriesSize;
}

/** @return The array entry at the given {@code index}. */
public Map.Entry<K, V> getArrayEntryAt(int index) {
return entryList.get(index);
if (index >= entriesSize) {
throw new ArrayIndexOutOfBoundsException(index);
}
@SuppressWarnings("unchecked")
Entry e = (Entry) entries[index];
return e;
}

/** @return There number of overflow entries. */
Expand All @@ -165,7 +168,7 @@ public Iterable<Map.Entry<K, V>> getOverflowEntries() {

@Override
public int size() {
return entryList.size() + overflowEntries.size();
return entriesSize + overflowEntries.size();
}

/**
Expand All @@ -191,7 +194,9 @@ public V get(Object o) {
final K key = (K) o;
final int index = binarySearchInArray(key);
if (index >= 0) {
return entryList.get(index).getValue();
@SuppressWarnings("unchecked")
Entry e = (Entry) entries[index];
return e.getValue();
}
return overflowEntries.get(key);
}
Expand All @@ -202,7 +207,9 @@ public V put(K key, V value) {
final int index = binarySearchInArray(key);
if (index >= 0) {
// Replace existing array entry.
return entryList.get(index).setValue(value);
@SuppressWarnings("unchecked")
Entry e = (Entry) entries[index];
return e.setValue(value);
}
ensureEntryArrayMutable();
final int insertionPoint = -(index + 1);
Expand All @@ -211,20 +218,26 @@ public V put(K key, V value) {
return getOverflowEntriesMutable().put(key, value);
}
// Insert new Entry in array.
if (entryList.size() == DEFAULT_FIELD_MAP_ARRAY_SIZE) {
if (entriesSize == DEFAULT_FIELD_MAP_ARRAY_SIZE) {
// Shift the last array entry into overflow.
final Entry lastEntryInArray = entryList.remove(DEFAULT_FIELD_MAP_ARRAY_SIZE - 1);
@SuppressWarnings("unchecked")
final Entry lastEntryInArray = (Entry) entries[DEFAULT_FIELD_MAP_ARRAY_SIZE - 1];
entriesSize--;
getOverflowEntriesMutable().put(lastEntryInArray.getKey(), lastEntryInArray.getValue());
}
entryList.add(insertionPoint, new Entry(key, value));
System.arraycopy(
entries, insertionPoint, entries, insertionPoint + 1, entries.length - insertionPoint - 1);
entries[insertionPoint] = new Entry(key, value);
entriesSize++;
return null;
}

@Override
public void clear() {
checkMutable();
if (!entryList.isEmpty()) {
entryList.clear();
if (entriesSize != 0) {
entries = null;
entriesSize = 0;
}
if (!overflowEntries.isEmpty()) {
overflowEntries.clear();
Expand Down Expand Up @@ -256,12 +269,17 @@ public V remove(Object o) {

private V removeArrayEntryAt(int index) {
checkMutable();
final V removed = entryList.remove(index).getValue();
@SuppressWarnings("unchecked")
final V removed = ((Entry) entries[index]).getValue();
// shift items across
System.arraycopy(entries, index + 1, entries, index, entriesSize - index - 1);
entriesSize--;
if (!overflowEntries.isEmpty()) {
// Shift the first entry in the overflow to be the last entry in the
// array.
final Iterator<Map.Entry<K, V>> iterator = getOverflowEntriesMutable().entrySet().iterator();
entryList.add(new Entry(iterator.next()));
entries[entriesSize] = new Entry(iterator.next());
entriesSize++;
iterator.remove();
}
return removed;
Expand All @@ -274,13 +292,14 @@ private V removeArrayEntryAt(int index) {
*/
private int binarySearchInArray(K key) {
int left = 0;
int right = entryList.size() - 1;
int right = entriesSize - 1;

// Optimization: For the common case in which entries are added in
// ascending tag order, check the largest element in the array before
// doing a full binary search.
if (right >= 0) {
int cmp = key.compareTo(entryList.get(right).getKey());
@SuppressWarnings("unchecked")
int cmp = key.compareTo(((Entry) entries[right]).getKey());
if (cmp > 0) {
return -(right + 2); // Insert point is after "right".
} else if (cmp == 0) {
Expand All @@ -290,7 +309,8 @@ private int binarySearchInArray(K key) {

while (left <= right) {
int mid = (left + right) / 2;
int cmp = key.compareTo(entryList.get(mid).getKey());
@SuppressWarnings("unchecked")
int cmp = key.compareTo(((Entry) entries[mid]).getKey());
if (cmp < 0) {
right = mid - 1;
} else if (cmp > 0) {
Expand Down Expand Up @@ -344,11 +364,13 @@ private SortedMap<K, V> getOverflowEntriesMutable() {
return (SortedMap<K, V>) overflowEntries;
}

/** Lazily creates the entry list. Any code that adds to the list must first call this method. */
/**
* Lazily creates the entry array. Any code that adds to the array must first call this method.
*/
private void ensureEntryArrayMutable() {
checkMutable();
if (entryList.isEmpty() && !(entryList instanceof ArrayList)) {
entryList = new ArrayList<>(DEFAULT_FIELD_MAP_ARRAY_SIZE);
if (entries == null) {
entries = new Object[DEFAULT_FIELD_MAP_ARRAY_SIZE];
}
}

Expand Down Expand Up @@ -498,7 +520,7 @@ private class EntryIterator implements Iterator<Map.Entry<K, V>> {

@Override
public boolean hasNext() {
return (pos + 1) < entryList.size()
return (pos + 1) < entriesSize
|| (!overflowEntries.isEmpty() && getOverflowIterator().hasNext());
}

Expand All @@ -507,8 +529,10 @@ public Map.Entry<K, V> next() {
nextCalledBeforeRemove = true;
// Always increment pos so that we know whether the last returned value
// was from the array or from overflow.
if (++pos < entryList.size()) {
return entryList.get(pos);
if (++pos < entriesSize) {
@SuppressWarnings("unchecked")
Entry e = (Entry) entries[pos];
return e;
}
return getOverflowIterator().next();
}
Expand All @@ -521,7 +545,7 @@ public void remove() {
nextCalledBeforeRemove = false;
checkMutable();

if (pos < entryList.size()) {
if (pos < entriesSize) {
removeArrayEntryAt(pos--);
} else {
getOverflowIterator().remove();
Expand All @@ -547,20 +571,22 @@ private Iterator<Map.Entry<K, V>> getOverflowIterator() {
*/
private class DescendingEntryIterator implements Iterator<Map.Entry<K, V>> {

private int pos = entryList.size();
private int pos = entriesSize;
private Iterator<Map.Entry<K, V>> lazyOverflowIterator;

@Override
public boolean hasNext() {
return (pos > 0 && pos <= entryList.size()) || getOverflowIterator().hasNext();
return (pos > 0 && pos <= entriesSize) || getOverflowIterator().hasNext();
}

@Override
public Map.Entry<K, V> next() {
if (getOverflowIterator().hasNext()) {
return getOverflowIterator().next();
}
return entryList.get(--pos);
@SuppressWarnings("unchecked")
Entry e = (Entry) entries[--pos];
return e;
}

@Override
Expand Down Expand Up @@ -621,7 +647,7 @@ public int hashCode() {
int h = 0;
final int listSize = getNumArrayEntries();
for (int i = 0; i < listSize; i++) {
h += entryList.get(i).hashCode();
h += entries[i].hashCode();
}
// Avoid the iterator allocation if possible.
if (getNumOverflowEntries() > 0) {
Expand Down

0 comments on commit 910f627

Please sign in to comment.