Skip to content

Commit 52e1bf1

Browse files
david-mollitor-dbIsaac
andcommitted
[SPARK-59431][CORE] Use ArrayList instead of LinkedList for BytesToBytesMap dataPages
`BytesToBytesMap` tracked its allocated data pages in a `LinkedList<MemoryBlock>`. Every operation on this field is an append at the end, a full iteration, an operation at the end (peek/remove last), or an index access -- there are no head or middle insertions/removals. That access pattern fits `ArrayList` better than `LinkedList`: contiguous storage (better iteration locality), no per-page `Node` allocation, and O(1) `get(index)` (used in the destructive `MapIterator` page advance). Change `dataPages` to an `ArrayList`. The `getLast()` / `removeLast()` calls (in `spill()` and `reset()`) are `SequencedCollection` methods available on `ArrayList` only since Java 21; since Spark supports Java 17 they are rewritten to `get(size() - 1)` / `remove(size() - 1)` (both O(1) for the last element). The `free()` drain, which used `Iterator.remove()` (O(n^2) on an `ArrayList`), is rewritten to remove from the end in a loop (O(n)), matching `reset()`. The sibling field `spillWriters` genuinely uses FIFO front removal (`getFirst()` / `removeFirst()`) and remains a `LinkedList`; only `dataPages` changes. No user-facing change: identical behavior; thread-safety is unchanged (access is guarded by the same external `synchronized` blocks). Generated-by: Claude Opus 4.8 Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 0edaeb8 commit 52e1bf1

1 file changed

Lines changed: 8 additions & 9 deletions

File tree

core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import javax.annotation.Nullable;
2121
import java.io.File;
2222
import java.io.IOException;
23+
import java.util.ArrayList;
2324
import java.util.Iterator;
2425
import java.util.LinkedList;
2526

@@ -103,9 +104,9 @@ public interface KeyOperationsFactory {
103104
private final TaskMemoryManager taskMemoryManager;
104105

105106
/**
106-
* A linked list for tracking all allocated data pages so that we can free all of our memory.
107+
* A list for tracking all allocated data pages so that we can free all of our memory.
107108
*/
108-
private final LinkedList<MemoryBlock> dataPages = new LinkedList<>();
109+
private final ArrayList<MemoryBlock> dataPages = new ArrayList<>();
109110

110111
/**
111112
* The data page that will be used to store keys and values for new hashtable entries. When this
@@ -413,7 +414,7 @@ public synchronized long spill(long numBytes) throws IOException {
413414

414415
long released = 0L;
415416
while (dataPages.size() > 0) {
416-
MemoryBlock block = dataPages.getLast();
417+
MemoryBlock block = dataPages.get(dataPages.size() - 1);
417418
// The currentPage is used, cannot be released
418419
if (block == currentPage) {
419420
break;
@@ -435,7 +436,7 @@ public synchronized long spill(long numBytes) throws IOException {
435436
writer.close();
436437
spillWriters.add(writer);
437438

438-
dataPages.removeLast();
439+
dataPages.remove(dataPages.size() - 1);
439440
released += block.size();
440441
freePage(block);
441442

@@ -1030,10 +1031,8 @@ public void free() {
10301031
freeArray(longArray);
10311032
longArray = null;
10321033
}
1033-
Iterator<MemoryBlock> dataPagesIterator = dataPages.iterator();
1034-
while (dataPagesIterator.hasNext()) {
1035-
MemoryBlock dataPage = dataPagesIterator.next();
1036-
dataPagesIterator.remove();
1034+
while (!dataPages.isEmpty()) {
1035+
MemoryBlock dataPage = dataPages.remove(dataPages.size() - 1);
10371036
freePage(dataPage);
10381037
}
10391038
assert(dataPages.isEmpty());
@@ -1115,7 +1114,7 @@ public void reset() {
11151114
freeArray(longArray);
11161115
longArray = null;
11171116
while (dataPages.size() > 0) {
1118-
MemoryBlock dataPage = dataPages.removeLast();
1117+
MemoryBlock dataPage = dataPages.remove(dataPages.size() - 1);
11191118
freePage(dataPage);
11201119
}
11211120
allocate(initialCapacity);

0 commit comments

Comments
 (0)