dongjoon-hyun commented on code in PR #60196:
URL: https://github.com/apache/spark/pull/60196#discussion_r4503019880
##########
core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java:
##########
@@ -1037,18 +1095,20 @@ public boolean append(Object kbase, long koff, int
klen, Object vbase, long voff
* @return whether there is enough space to allocate the new page.
*/
private boolean acquireNewPage(long required) {
+ final long size = Math.max(nextPageSizeBytes, required);
final MemoryBlock page;
try {
- page = allocatePage(required);
+ page = allocatePage(size, required);
} catch (SparkOutOfMemoryError e) {
return false;
}
// Retaining exact-fit minimum-retry pages would consume one page-table
slot per map entry.
- if (required < pageSizeBytes && page.size() == required &&
+ if (required < size && page.size() == required &&
isPageAllocationFromMinimumRetry(page)) {
freePage(page);
return false;
}
+ nextPageSizeBytes = Math.min(pageSizeBytes, nextPageSizeBytes * 2);
Review Comment:
Good catch. The growing pages now stop at exactly one page size: the last of
them is cut to what is left to reach the page size, so with a 64 MiB page size
the pages are 1, 2, 4, 8, 16, 32 and 1 MiB before the 64 MiB pages. The total
is reset in `reset()`, and maps created through the existing constructors still
request the page size for every page. `dataPagesGrowFromInitialPageSize` now
uses a 6 KiB page size and checks that the growing pages are 1, 2 and 3 KiB
before the 6 KiB pages.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala:
##########
@@ -452,6 +453,17 @@ private[joins] class UnsafeHashedRelation(
private[joins] object UnsafeHashedRelation {
+ /**
+ * The size of the first data page of the map. Later pages double in size up
to the page size.
+ *
+ * Otherwise `BytesToBytesMap` rounds its first page up to the page size
(`spark.buffer.pageSize`,
+ * up to 64 MiB), so even a relation with a few rows reserves 64 MiB. A task
that keeps many
Review Comment:
Thanks, updated. The comment now says that even a few rows reserve a full
page of `spark.buffer.pageSize` or a default of up to 64 MiB.
##########
core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java:
##########
@@ -175,6 +175,17 @@ private enum MapState {
*/
private final long pageSizeBytes;
+ /**
+ * The size of the first data page, at most {@link #pageSizeBytes}.
Review Comment:
Updated. The `pageSizeBytes` doc now calls it the maximum size of regular
data pages, which a map with a smaller initial page size grows its pages up to,
and says that a record larger than a page gets a page of its own size.
##########
core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java:
##########
@@ -259,10 +290,37 @@ public BytesToBytesMap(
throw new IllegalArgumentException("Page size " + pageSizeBytes + "
cannot exceed " +
TaskMemoryManager.MAXIMUM_PAGE_SIZE_BYTES);
}
+ if (initialPageSizeBytes <= 0) {
+ throw new IllegalArgumentException("Initial page size must be greater
than 0");
+ }
+ this.initialPageSizeBytes = Math.min(initialPageSizeBytes, pageSizeBytes);
Review Comment:
Added. The test also checks that a map whose initial page size is larger
than its page size starts with a page of the page size.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala:
##########
@@ -469,7 +481,8 @@ private[joins] object UnsafeHashedRelation {
taskMemoryManager,
// Only 70% of the slots can be used before growing, more capacity help
to reduce collision
(sizeEstimate * 1.5 + 1).toInt,
- pageSizeBytes)
+ pageSizeBytes,
Review Comment:
Added a `private[joins]` accessor for the map's page size. The test now
checks that a built relation and its Java and Kryo copies grow their pages up
to the `SparkEnv` page size.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
