Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reference count on Java DeviceMemoryBuffer after contiguousSplit [skip ci] #5915

Merged
merged 2 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@
- PR #5872 libcudf_kafka r_path is causing docker build failures on centos7
- PR #5869 Fix bug in parquet writer in writing string column with offset
- PR #5895 Do not break kafka client consumption loop on local client timeout
- PR #5915 Fix reference count on Java DeviceMemoryBuffer after contiguousSplit


# cuDF 0.14.0 (03 Jun 2020)
Expand Down
2 changes: 2 additions & 0 deletions java/src/main/java/ai/rapids/cudf/ColumnVector.java
Original file line number Diff line number Diff line change
Expand Up @@ -3079,6 +3079,8 @@ public OffHeapState(long viewHandle, DeviceMemoryBuffer contiguousBuffer) {
toClose.add(data);
toClose.add(valid);
toClose.add(offsets);
contiguousBuffer.incRefCount();
toClose.add(contiguousBuffer);
}

public long getViewHandle() {
Expand Down
6 changes: 2 additions & 4 deletions java/src/main/java/ai/rapids/cudf/DeviceMemoryBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ public static DeviceMemoryBuffer allocate(long bytes) {
@Override
public synchronized final DeviceMemoryBuffer slice(long offset, long len) {
addressOutOfBoundsCheck(address + offset, len, "slice");
refCount++;
cleaner.addRef();
incRefCount();
return new DeviceMemoryBuffer(getAddress() + offset, len, this);
}

Expand All @@ -151,8 +150,7 @@ synchronized final BaseDeviceMemoryBuffer sliceFrom(DeviceMemoryBufferView view)
return null;
}
addressOutOfBoundsCheck(view.address, view.length, "sliceFrom");
refCount++;
cleaner.addRef();
incRefCount();
return new DeviceMemoryBuffer(view.address, view.length, this);
}
}
17 changes: 15 additions & 2 deletions java/src/main/java/ai/rapids/cudf/MemoryBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ protected MemoryBuffer(long address, long length, MemoryBufferCleaner cleaner) {
this.cleaner = cleaner;
if (cleaner != null) {
this.id = cleaner.id;
refCount++;
cleaner.addRef();
incRefCount();
MemoryCleaner.register(this, cleaner);
} else {
this.id = -1;
Expand Down Expand Up @@ -190,4 +189,18 @@ public String toString() {
", length=" + length +
", id=" + id + "}";
}

/**
* Increment the reference count for this column. You need to call close on this
* to decrement the reference count again.
*/
public synchronized void incRefCount() {
refCount++;
cleaner.addRef();
}

// visible for testing
synchronized int getRefCount() {
return refCount;
}
}
9 changes: 9 additions & 0 deletions java/src/test/java/ai/rapids/cudf/ColumnVectorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2829,4 +2829,13 @@ void testConcatListsOfLists() {
assertColumnsAreEqual(expected, v);
}
}

@Test
void testContiguousSplitConstructor() {
try (Table tmp = new Table.TestBuilder().column(1, 2).column(3, 4).build();
ContiguousTable ct = tmp.contiguousSplit()[0]) {
// one reference for the device buffer itself, two more for the column using it
assertEquals(3, ct.getBuffer().getRefCount());
}
}
}