shuwenwei commented on code in PR #18409:
URL: https://github.com/apache/iotdb/pull/18409#discussion_r3754863147
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/datastructure/AlignedTVList.java:
##########
@@ -212,39 +269,190 @@ public synchronized AlignedTVList cloneForFlushSort() {
public synchronized AlignedTVList clone() {
AlignedTVList cloneList = AlignedTVList.newAlignedList(new
ArrayList<>(dataTypes));
cloneAs(cloneList);
- cloneList.timeDeletedCnt = this.timeDeletedCnt;
- System.arraycopy(
- memoryBinaryChunkSize, 0, cloneList.memoryBinaryChunkSize, 0,
dataTypes.size());
+ cloneColumnDataTo(cloneList, null);
+ cloneList.materializedValueArrayCounts =
+ Arrays.copyOf(materializedValueArrayCounts,
materializedValueArrayCounts.length);
+ cloneList.materializedValueArrayMemCost = materializedValueArrayMemCost;
+ return cloneList;
+ }
+
+ /**
+ * Prepare a partial clone without changing this TVList. The returned plan
must be committed only
+ * after the query-memory reservation succeeds.
+ */
+ public synchronized PartialClonePlan preparePartialClone(Set<Integer>
columnsToClone) {
+ Set<Integer> retainedColumns =
+ new HashSet<>(
+ Objects.requireNonNull(
+ columnsToClone,
+
DataNodeMiscMessages.EXCEPTION_COLUMNSTOCLONE_CANNOT_BE_NULL_458FDF37));
+ AlignedTVList cloneList = AlignedTVList.newAlignedList(new
ArrayList<>(dataTypes), false);
+ // Pre-create the inner value lists for the retained columns; the other
slots stay null until
+ // the ownership transfer moves the source columns into place.
for (int i = 0; i < values.size(); i++) {
- // Clone value
+ if (retainedColumns.contains(i)) {
+ cloneList.values.set(i, new ArrayList<>(values.get(i).size()));
+ }
+ }
+ cloneAs(cloneList);
+ cloneColumnDataTo(cloneList, retainedColumns);
+ return prepareMovePlan(cloneList, retainedColumns);
+ }
+
+ @SuppressWarnings("unchecked")
+ private PartialClonePlan prepareMovePlan(AlignedTVList cloneList,
Set<Integer> retainedColumns) {
+ Objects.requireNonNull(
+ cloneList,
DataNodeMiscMessages.EXCEPTION_CLONELIST_CANNOT_BE_NULL_47AEEA8F);
+ int columnCount = values.size();
+ if (cloneList.values.size() != columnCount
+ || cloneList.memoryBinaryChunkSize.length !=
memoryBinaryChunkSize.length) {
+ throw new IllegalStateException(
+ DataNodeMiscMessages
+
.EXCEPTION_TARGET_ALIGNEDTVLIST_HAS_INCOMPATIBLE_COLUMN_CONTAINERS_31FAC613);
+ }
+
+ List<Object>[] valueColumnsToMove = (List<Object>[]) new
List<?>[columnCount];
+ List<BitMap>[] bitmapColumnsToMove = (List<BitMap>[]) new
List<?>[columnCount];
Review Comment:
Fixed in a35f0763 by taking the "eliminate these arrays" option:
`prepareMovePlan` now validates the move without allocating any O(N) structure
(all existing checks are kept), and `commitPartialClone` derives the moved
columns from the retained set (`retainedColumns.contains(i) == false`), so the
commit is fully allocation-free.
On the question of whether the two arrays deserve a separate reservation:
after the earlier change (f7df7b689c) removed the N per-column inner ArrayLists
by creating the clone with null outer slots and pre-creating inner lists only
for retained columns, these two `columnCount` reference arrays (2 x N x
NUM_BYTES_OBJECT_REF) were the only remaining transient allocation between the
reservation and the commit. They are now gone entirely, so the preparation
phase between `reserveMemoryCumulatively(listRamInfo)` and `commit()` is
allocation-free and the admission-control claim holds by construction — there
is no temporary O(N) state left to reserve or release. The existing
partial-clone and end-to-end FragmentInstanceExecution tests exercise
preparePartialClone + commit on sparse/wide layouts and pass.
--
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]