Optimize Buffer.fill() to avoid intermediate object creation - #1376
Merged
Conversation
Use Cython typed parameters and C buffer API to eliminate overhead: - int path: uint8_t parameter for automatic overflow checking - bytes path: direct char* pointer access - general buffer path: PyObject_GetBuffer for direct void* access
Andy-Jost
added
enhancement
Contributor
Contributor
Author
|
/ok to test 415469e |
Contributor
Author
|
@kkraus14 I think this implementation follows the approach you outlined. |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/ok to test 355be58 |
kkraus14
approved these changes
Dec 16, 2025
Collaborator
|
LGTM, this was basically exactly what I had in mind except your implementation was more elegantly using Cython instead of jumping straight down to the CPython API 😄 |
leofang
approved these changes
Dec 16, 2025
leofang
left a comment
leofang
left a comment
Member
There was a problem hiding this comment.
LGTM too! Left a few nits. Nothing is critical.
Comment on lines
+385
to
+387
| cdef inline void Buffer_fill_uint8(Buffer self, uint8_t value, cydriver.CUstream s): | ||
| with nogil: | ||
| HANDLE_RETURN(cydriver.cuMemsetD8Async(<cydriver.CUdeviceptr>self._ptr, value, self._size, s)) |
Member
There was a problem hiding this comment.
nit: we want the exception clause except* since HANDLE_RETURN can raise, but we don't want Cython to warn, so with Cython 3 we general want to avoid using void as the return type and use this instead:
Suggested change
| cdef inline void Buffer_fill_uint8(Buffer self, uint8_t value, cydriver.CUstream s): | |
| with nogil: | |
| HANDLE_RETURN(cydriver.cuMemsetD8Async(<cydriver.CUdeviceptr>self._ptr, value, self._size, s)) | |
| cdef inline int Buffer_fill_uint8(Buffer self, uint8_t value, cydriver.CUstream s) except?-1: | |
| with nogil: | |
| HANDLE_RETURN(cydriver.cuMemsetD8Async(<cydriver.CUdeviceptr>self._ptr, value, self._size, s)) | |
| return 0 |
Comment on lines
+390
to
+392
| cdef inline void Buffer_fill_from_ptr( | ||
| Buffer self, const char* ptr, size_t width, cydriver.CUstream s | ||
| ) except *: |
Member
There was a problem hiding this comment.
ditto
Suggested change
| cdef inline void Buffer_fill_from_ptr( | |
| Buffer self, const char* ptr, size_t width, cydriver.CUstream s | |
| ) except *: | |
| cdef inline int Buffer_fill_from_ptr( | |
| Buffer self, const char* ptr, size_t width, cydriver.CUstream s | |
| ) except?-1: |
| HANDLE_RETURN(cydriver.cuMemsetD32Async( | ||
| <cydriver.CUdeviceptr>self._ptr, (<uint32_t*>ptr)[0], buffer_size // 4, s)) | ||
| else: | ||
| raise ValueError(f"value must be 1, 2, or 4 bytes, got {width}") |
Member
There was a problem hiding this comment.
Suggested change
| raise ValueError(f"value must be 1, 2, or 4 bytes, got {width}") | |
| raise ValueError(f"value must be 1, 2, or 4 bytes, got {width}") | |
| return 0 |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Buffer.fill()uint8_tparameter for automatic overflow checkingchar*pointer accessPyObject_GetBufferfor directvoid*accessCloses #1375
Test Plan
test_buffer_filltests pass