This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 7c75efcbc17a [SPARK-57684][CORE] Avoid throwaway allocation in
InMemoryStore parent-index lookups
7c75efcbc17a is described below
commit 7c75efcbc17aec1458602a17199276a5212a6ec8
Author: YangJie <[email protected]>
AuthorDate: Thu Jun 25 14:40:42 2026 +0800
[SPARK-57684][CORE] Avoid throwaway allocation in InMemoryStore
parent-index lookups
### What changes were proposed in this pull request?
`InMemoryStore` looked up children at two spots with
`parentToChildrenMap.getOrDefault(parentKey, new NaturalKeys())`. Java
evaluates the default argument eagerly, so a fresh empty `NaturalKeys` was
allocated on every call, even when the key was present. Both spots only read
the result (they iterate its keys), so this switches to `get(parentKey)` with a
null guard: `removeAllByIndexValues` continues to the next value on a miss, and
`InMemoryView.copyElements` returns an empty list. On [...]
### Why are the changes needed?
It drops an allocation per call on the delete and child-view paths. The
previous `getOrDefault` form built and discarded an empty map on every lookup,
including hits.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added two `InMemoryStoreSuite` tests for the natural-parent-index paths,
which were not directly covered before: `testRemoveAllByNaturalParentIndex`
(removing by a parent value, plus an absent parent that removes nothing) and
`testViewByNaturalParentIndex` (a parent-filtered view returning its children
in order, plus an absent parent yielding an empty iterator). The full kvstore
suite passes, with the LevelDB suites skipped on Apple Silicon and run on CI,
and checkstyle reports no vio [...]
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #56759 from LuciferYang/SPARK-kvstore-inmemory-alloc.
Authored-by: YangJie <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
---
.../apache/spark/util/kvstore/InMemoryStore.java | 10 ++-
.../spark/util/kvstore/InMemoryStoreSuite.java | 79 ++++++++++++++++++++++
2 files changed, 87 insertions(+), 2 deletions(-)
diff --git
a/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
b/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
index bb9c68bc8a96..4b83c045690a 100644
---
a/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
+++
b/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
@@ -249,7 +249,10 @@ public class InMemoryStore implements KVStore {
// delete them from `data`.
for (Object indexValue : indexValues) {
Comparable<Object> parentKey = asKey(indexValue);
- NaturalKeys children = parentToChildrenMap.getOrDefault(parentKey,
new NaturalKeys());
+ NaturalKeys children = parentToChildrenMap.get(parentKey);
+ if (children == null) {
+ continue;
+ }
for (Comparable<Object> naturalKey : children.keySet()) {
data.remove(naturalKey);
count ++;
@@ -413,7 +416,10 @@ public class InMemoryStore implements KVStore {
// If there is a parent index for the natural index and the parent
of `index` happens to
// be it, Spark can use the `parentToChildrenMap` to get the related
natural keys, and
// then copy them from `data`.
- NaturalKeys children = parentToChildrenMap.getOrDefault(parentKey,
new NaturalKeys());
+ NaturalKeys children = parentToChildrenMap.get(parentKey);
+ if (children == null) {
+ return new ArrayList<>();
+ }
ArrayList<T> elements = new ArrayList<>();
for (Comparable<Object> naturalKey : children.keySet()) {
data.computeIfPresent(naturalKey, (k, v) -> {
diff --git
a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/InMemoryStoreSuite.java
b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/InMemoryStoreSuite.java
index ed4e58c974af..e17b7c2adbd6 100644
---
a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/InMemoryStoreSuite.java
+++
b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/InMemoryStoreSuite.java
@@ -238,6 +238,85 @@ public class InMemoryStoreSuite {
assertEquals(0, store.count(CustomType2.class));
}
+ @Test
+ public void testRemoveAllByNaturalParentIndex() throws Exception {
+ KVStore store = new InMemoryStore();
+
+ CustomType2 t1 = new CustomType2();
+ t1.key = "key1";
+ t1.id = "id1";
+ t1.parentId = "parentId1";
+ store.write(t1);
+
+ CustomType2 t2 = new CustomType2();
+ t2.key = "key2";
+ t2.id = "id2";
+ t2.parentId = "parentId1";
+ store.write(t2);
+
+ CustomType2 t3 = new CustomType2();
+ t3.key = "key3";
+ t3.id = "id3";
+ t3.parentId = "parentId2";
+ store.write(t3);
+
+ assertEquals(3, store.count(CustomType2.class));
+
+ // A parent value with no children removes nothing.
+ assertFalse(store.removeAllByIndexValues(
+ CustomType2.class, "parentId", Set.of("noSuchParent")));
+ assertEquals(3, store.count(CustomType2.class));
+
+ // Removing by a parent value deletes all of its children, and only those.
+ assertTrue(store.removeAllByIndexValues(
+ CustomType2.class, "parentId", Set.of("parentId1")));
+ assertEquals(1, store.count(CustomType2.class));
+ assertEquals("id3", store.read(CustomType2.class, "key3").id);
+
+ assertTrue(store.removeAllByIndexValues(
+ CustomType2.class, "parentId", Set.of("parentId2")));
+ assertEquals(0, store.count(CustomType2.class));
+ }
+
+ @Test
+ public void testViewByNaturalParentIndex() throws Exception {
+ KVStore store = new InMemoryStore();
+
+ CustomType2 t1 = new CustomType2();
+ t1.key = "key1";
+ t1.id = "id1";
+ t1.parentId = "parentId1";
+ store.write(t1);
+
+ CustomType2 t2 = new CustomType2();
+ t2.key = "key2";
+ t2.id = "id2";
+ t2.parentId = "parentId1";
+ store.write(t2);
+
+ CustomType2 t3 = new CustomType2();
+ t3.key = "key3";
+ t3.id = "id3";
+ t3.parentId = "parentId2";
+ store.write(t3);
+
+ // A populated parent returns exactly its children, in natural-key order.
+ try (KVStoreIterator<CustomType2> it =
+ store.view(CustomType2.class).parent("parentId1").closeableIterator())
{
+ assertTrue(it.hasNext());
+ assertEquals("key1", it.next().key);
+ assertTrue(it.hasNext());
+ assertEquals("key2", it.next().key);
+ assertFalse(it.hasNext());
+ }
+
+ // A parent value with no children yields an empty iterator (the
early-return branch).
+ try (KVStoreIterator<CustomType2> it =
+
store.view(CustomType2.class).parent("noSuchParent").closeableIterator()) {
+ assertFalse(it.hasNext());
+ }
+ }
+
@Test
public void testCountByIndexValue() throws Exception {
KVStore store = new InMemoryStore();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
