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

Exposure Tracked Buffer (first step towards unifying copy-on-write and spilling) #13307

Merged
merged 32 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
09b22ab
Impl. TenableBuffer
madsbk May 8, 2023
b43c8cc
doc
madsbk May 9, 2023
d27bb25
remove _get_cuda_array_interface
madsbk May 9, 2023
38e3c97
removed get_raw_ptr
madsbk May 9, 2023
f091326
as_tenable_buffer(): accept subclass
madsbk May 9, 2023
b5027ff
cleanup
madsbk May 9, 2023
13e3d2a
fix TenableBuffer check
madsbk May 9, 2023
0848ab8
doc
madsbk May 10, 2023
a924926
clean up
madsbk May 10, 2023
122bc70
doc
madsbk May 10, 2023
0debdcd
spelling and typos
madsbk May 10, 2023
e7a44ac
get_ptr(): make `mode` mandatory
madsbk May 10, 2023
d9fc54e
remove BufferSlice.exposed
madsbk May 10, 2023
a9c320e
from_column_view(): mark_exposed when the relationship isn't known
madsbk May 10, 2023
8e26ee2
Merge branch 'branch-23.06' of github.com:rapidsai/cudf into tenable_…
madsbk May 10, 2023
3b63e9d
doc
madsbk May 11, 2023
1c13953
cleanup
madsbk May 11, 2023
d1b49ee
get_ptr: typing mode literal
madsbk May 11, 2023
3964c1f
Merge branch 'branch-23.06' of github.com:rapidsai/cudf into tenable_…
madsbk May 11, 2023
350b95d
Merge branch 'branch-23.06' of github.com:rapidsai/cudf into tenable_…
madsbk May 15, 2023
38b4ba4
use Self
madsbk May 15, 2023
df53860
Merge branch 'branch-23.06' of github.com:rapidsai/cudf into tenable_…
madsbk May 15, 2023
5ec77f5
Merge branch 'branch-23.06' of github.com:rapidsai/cudf into tenable_…
madsbk Jun 19, 2023
1b67bd8
Merge branch 'branch-23.08' of github.com:rapidsai/cudf into tenable_…
madsbk Jun 19, 2023
3b5f56e
rename: tenable -> exposure tracked
madsbk Jun 19, 2023
2056c6f
doc
madsbk Jun 19, 2023
8a8fd93
spelling
madsbk Jun 23, 2023
f30e11f
added a TODO
madsbk Jun 30, 2023
98581a8
Merge branch 'branch-23.08' of github.com:rapidsai/cudf into tenable_…
madsbk Jun 30, 2023
e4713db
Merge branch 'tenable_buffer' of github.com:madsbk/cudf into tenable_…
madsbk Jun 30, 2023
e21b66b
typo
madsbk Jul 3, 2023
c020dbd
Merge branch 'branch-23.08' into tenable_buffer
madsbk Jul 3, 2023
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
26 changes: 11 additions & 15 deletions docs/cudf/source/developer_guide/library_design.md
Original file line number Diff line number Diff line change
Expand Up @@ -325,30 +325,26 @@ This section describes the internal implementation details of the copy-on-write
It is recommended that developers familiarize themselves with [the user-facing documentation](copy-on-write-user-doc) of this functionality before reading through the internals
below.

The core copy-on-write implementation relies on the `CopyOnWriteBuffer` class.
When the cudf option `"copy_on_write"` is `True`, `as_buffer` will always return a `CopyOnWriteBuffer`.
This subclass of `cudf.Buffer` contains all the mechanisms to enable copy-on-write behavior.
The class stores [weak references](https://docs.python.org/3/library/weakref.html) to every existing `CopyOnWriteBuffer` in `CopyOnWriteBuffer._instances`, a mapping from `ptr` keys to `WeakSet`s containing references to `CopyOnWriteBuffer` objects.
This means that all `CopyOnWriteBuffer`s that point to the same device memory are contained in the same `WeakSet` (corresponding to the same `ptr` key) in `CopyOnWriteBuffer._instances`.
This data structure is then used to determine whether or not to make a copy when a write operation is performed on a `Column` (see below).
If multiple buffers point to the same underlying memory, then a copy must be made whenever a modification is attempted.
The core copy-on-write implementation relies on the factory function `as_exposure_tracked_buffer` and the two classes `ExposureTrackedBuffer` and `BufferSlice`.

A `ExposureTrackedBuffer` is a subclass of the regular `Buffer` that tracks internal and external references to its underlying memory. Internal references are tracked by maintaining [weak references](https://docs.python.org/3/library/weakref.html) to every `BufferSlice` of the underlying memory. External references are tracked through "exposure" status of the underlying memory. A buffer is considered exposed if the device pointer (integer or void*) has been handed out to a library outside of cudf. In this case, we have no way of knowing if the data are being modified by a third party.
madsbk marked this conversation as resolved.
Show resolved Hide resolved

`BufferSlice` is a subclass of `ExposureTrackedBuffer` that represents a _slice_ of the memory underlying a exposure tracked buffer.

When the cudf option `"copy_on_write"` is `True`, `as_buffer` calls `as_exposure_tracked_buffer`, which always returns a `BufferSlice`. It is then the slices that determine whether or not to make a copy when a write operation is performed on a `Column` (see below). If multiple slices point to the same underlying memory, then a copy must be made whenever a modification is attempted.


### Eager copies when exposing to third-party libraries

If a `Column`/`CopyOnWriteBuffer` is exposed to a third-party library via `__cuda_array_interface__`, we are no longer able to track whether or not modification of the buffer has occurred. Hence whenever
If a `Column`/`BufferSlice` is exposed to a third-party library via `__cuda_array_interface__`, we are no longer able to track whether or not modification of the buffer has occurred. Hence whenever
someone accesses data through the `__cuda_array_interface__`, we eagerly trigger the copy by calling
`_unlink_shared_buffers` which ensures a true copy of underlying device data is made and
unlinks the buffer from any shared "weak" references. Any future copy requests must also trigger a true physical copy (since we cannot track the lifetime of the third-party object). To handle this we also mark the `Column`/`CopyOnWriteBuffer` as
`obj._zero_copied=True` thus indicating that any future shallow-copy requests will trigger a true physical copy
rather than a copy-on-write shallow copy with weak references.
`.make_single_owner_inplace` which ensures a true copy of underlying data is made and that the slice is the sole owner. Any future copy requests must also trigger a true physical copy (since we cannot track the lifetime of the third-party object). To handle this we also mark the `Column`/`BufferSlice` as exposed thus indicating that any future shallow-copy requests will trigger a true physical copy rather than a copy-on-write shallow copy.

### Obtaining a read-only object

A read-only object can be quite useful for operations that will not
mutate the data. This can be achieved by calling `._get_cuda_array_interface(readonly=True)`, and creating a `SimpleNameSpace` object around it.
This will not trigger a deep copy even if the `CopyOnWriteBuffer`
has weak references. This API should only be used when the lifetime of the proxy object is restricted to cudf's internal code execution. Handing this out to external libraries or user-facing APIs will lead to untracked references and undefined copy-on-write behavior. We currently use this API for device to host
mutate the data. This can be achieved by calling `.get_ptr(mode="read")`, and using `cuda_array_interface_wrapper` to wrap a `__cuda_array_interface__` object around it.
This will not trigger a deep copy even if multiple `BufferSlice` points to the same `ExposureTrackedBuffer`. This API should only be used when the lifetime of the proxy object is restricted to cudf's internal code execution. Handing this out to external libraries or user-facing APIs will lead to untracked references and undefined copy-on-write behavior. We currently use this API for device to host
copies like in `ColumnBase.data_array_view(mode="read")` which is used for `Column.values_host`.


Expand Down
29 changes: 16 additions & 13 deletions python/cudf/cudf/_lib/column.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import cudf._lib as libcudf
from cudf.api.types import is_categorical_dtype, is_datetime64tz_dtype
from cudf.core.buffer import (
Buffer,
CopyOnWriteBuffer,
ExposureTrackedBuffer,
SpillableBuffer,
acquire_spill_lock,
as_buffer,
Expand Down Expand Up @@ -339,8 +339,8 @@ cdef class Column:
if col.base_data is None:
data = NULL
else:
data = <void*><uintptr_t>(col.base_data.get_ptr(
mode="write")
data = <void*><uintptr_t>(
col.base_data.get_ptr(mode="write")
)

cdef Column child_column
Expand Down Expand Up @@ -534,13 +534,16 @@ cdef class Column:
rmm.DeviceBuffer(ptr=data_ptr,
size=(size+offset) * dtype_itemsize)
)
elif column_owner and isinstance(data_owner, CopyOnWriteBuffer):
# TODO: In future, see if we can just pass on the
# CopyOnWriteBuffer reference to another column
# and still create a weak reference.
# With the current design that's not possible.
# https://github.com/rapidsai/cudf/issues/12734
vyasr marked this conversation as resolved.
Show resolved Hide resolved
data = data_owner.copy(deep=False)
elif (
column_owner and
isinstance(data_owner, ExposureTrackedBuffer)
):
data = as_buffer(
data=data_ptr,
size=base_nbytes,
owner=data_owner,
exposed=False,
)
elif (
# This is an optimization of the most common case where
# from_column_view creates a "view" that is identical to
Expand All @@ -564,9 +567,9 @@ cdef class Column:
owner=data_owner,
exposed=True,
)
if isinstance(data_owner, CopyOnWriteBuffer):
data_owner.get_ptr(mode="write")
madsbk marked this conversation as resolved.
Show resolved Hide resolved
# accessing the pointer marks it exposed.
if isinstance(data_owner, ExposureTrackedBuffer):
# accessing the pointer marks it exposed permanently.
data_owner.mark_exposed()
elif isinstance(data_owner, SpillableBuffer):
if data_owner.is_spilled:
raise ValueError(
Expand Down
2 changes: 1 addition & 1 deletion python/cudf/cudf/core/buffer/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright (c) 2022-2023, NVIDIA CORPORATION.

from cudf.core.buffer.buffer import Buffer, cuda_array_interface_wrapper
from cudf.core.buffer.cow_buffer import CopyOnWriteBuffer
from cudf.core.buffer.exposure_tracked_buffer import ExposureTrackedBuffer
from cudf.core.buffer.spillable_buffer import SpillableBuffer, SpillLock
from cudf.core.buffer.utils import (
acquire_spill_lock,
Expand Down
47 changes: 17 additions & 30 deletions python/cudf/cudf/core/buffer/buffer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import math
import pickle
from types import SimpleNamespace
from typing import Any, Dict, Mapping, Optional, Sequence, Tuple
from typing import Any, Dict, Literal, Mapping, Optional, Sequence, Tuple

import numpy
from typing_extensions import Self
Expand Down Expand Up @@ -168,7 +168,7 @@ def _from_host_memory(cls, data: Any) -> Self:
# Create from device memory
return cls._from_device_memory(buf)

def _getitem(self, offset: int, size: int) -> Buffer:
def _getitem(self, offset: int, size: int) -> Self:
"""
Sub-classes can overwrite this to implement __getitem__
without having to handle non-slice inputs.
Expand All @@ -181,7 +181,7 @@ def _getitem(self, offset: int, size: int) -> Buffer:
)
)

def __getitem__(self, key: slice) -> Buffer:
def __getitem__(self, key: slice) -> Self:
"""Create a new slice of the buffer."""
if not isinstance(key, slice):
raise TypeError(
Expand All @@ -193,7 +193,7 @@ def __getitem__(self, key: slice) -> Buffer:
raise ValueError("slice must be C-contiguous")
return self._getitem(offset=start, size=stop - start)

def copy(self, deep: bool = True):
def copy(self, deep: bool = True) -> Self:
"""
Return a copy of Buffer.

Expand Down Expand Up @@ -233,35 +233,15 @@ def owner(self) -> Any:
@property
def __cuda_array_interface__(self) -> Mapping:
"""Implementation of the CUDA Array Interface."""
return self._get_cuda_array_interface(readonly=False)

def _get_cuda_array_interface(self, readonly=False):
"""Helper function to create a CUDA Array Interface.

Parameters
----------
readonly : bool, default False
If True, returns a CUDA Array Interface with
readonly flag set to True.
If False, returns a CUDA Array Interface with
readonly flag set to False.

Returns
-------
dict
"""
return {
"data": (
self.get_ptr(mode="read" if readonly else "write"),
readonly,
),
"data": (self.get_ptr(mode="write"), False),
"shape": (self.size,),
"strides": None,
"typestr": "|u1",
"version": 0,
}

def get_ptr(self, *, mode) -> int:
def get_ptr(self, *, mode: Literal["read", "write"]) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change this, but just to note that Literal doesn't get handled very well by type-checkers if the argument comes from a variable, rather than a literal value (unless the variable is marked with : Final). Since they don't do dataflow analysis

https://mypy-play.net/?mypy=latest&python=3.11&gist=6541ee12e80daeb4b0837563d98dc442

"""Device pointer to the start of the buffer.

Parameters
Expand All @@ -274,19 +254,26 @@ def get_ptr(self, *, mode) -> int:
Failure to fulfill this contract will cause
incorrect behavior.

Returns
-------
int
The device pointer as an integer

See Also
--------
SpillableBuffer.get_ptr
CopyOnWriteBuffer.get_ptr
ExposureTrackedBuffer.get_ptr
"""
return self._ptr

def memoryview(self) -> memoryview:
def memoryview(
self, *, offset: int = 0, size: Optional[int] = None
) -> memoryview:
"""Read-only access to the buffer through host memory."""
host_buf = host_memory_allocation(self.size)
size = self._size if size is None else size
host_buf = host_memory_allocation(size)
rmm._lib.device_buffer.copy_ptr_to_host(
self.get_ptr(mode="read"), host_buf
self.get_ptr(mode="read") + offset, host_buf
)
wence- marked this conversation as resolved.
Show resolved Hide resolved
return memoryview(host_buf).toreadonly()

Expand Down
168 changes: 0 additions & 168 deletions python/cudf/cudf/core/buffer/cow_buffer.py

This file was deleted.

Loading