Skip to content

Commit

Permalink
MAINT: Remove buffer-clearing from copy code
Browse files Browse the repository at this point in the history
Buffer must always either contain NULL or valid references
(assuming the dtype supports references) in order to allow
cleanup in case of errors.

It is thus unnecessary to clear buffers before every copy, if
they contain NULL, all is fine. If they contain non-NULL, we should
also DECREF those references (so it would be incorrect as well).

Buffers thus need the memset exactly once: directly upon allcoation
(which we do now).  After this any transfer from/to the buffer needs
to ensure that the buffer is always in a good state.
  • Loading branch information
seberg authored and charris committed May 6, 2021
1 parent 92da57b commit acf6648
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 22 deletions.
32 changes: 12 additions & 20 deletions numpy/core/src/multiarray/nditer_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -2542,16 +2542,18 @@ npyiter_copy_to_buffers(NpyIter *iter, char **prev_dataptrs)
skip_transfer = 1;
}

/* If the data type requires zero-inititialization */
if (PyDataType_FLAGCHK(dtypes[iop], NPY_NEEDS_INIT)) {
NPY_IT_DBG_PRINT("Iterator: Buffer requires init, "
"memsetting to 0\n");
memset(ptrs[iop], 0, dtypes[iop]->elsize*op_transfersize);
/* Can't skip the transfer in this case */
skip_transfer = 0;
}

if (!skip_transfer) {
/*
* Copy data to the buffers if necessary.
*
* We always copy if the operand has references. In that case
* a "write" function must be in use that either copies or clears
* the buffer.
* This write from buffer call does not check for skip-transfer
* so we have to assume the buffer is cleared. For dtypes that
* do not have references, we can assume that the write function
* will leave the source (buffer) unmodified.
*/
if (!skip_transfer || PyDataType_REFCHK(dtypes[iop])) {
NPY_IT_DBG_PRINT2("Iterator: Copying operand %d to "
"buffer (%d items)\n",
(int)iop, (int)op_transfersize);
Expand All @@ -2567,16 +2569,6 @@ npyiter_copy_to_buffers(NpyIter *iter, char **prev_dataptrs)
}
}
}
else if (ptrs[iop] == buffers[iop]) {
/* If the data type requires zero-inititialization */
if (PyDataType_FLAGCHK(dtypes[iop], NPY_NEEDS_INIT)) {
NPY_IT_DBG_PRINT1("Iterator: Write-only buffer for "
"operand %d requires init, "
"memsetting to 0\n", (int)iop);
memset(ptrs[iop], 0, dtypes[iop]->elsize*transfersize);
}
}

}

/*
Expand Down
4 changes: 2 additions & 2 deletions numpy/core/tests/test_nditer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2748,8 +2748,8 @@ def __bool__(self):

def test_object_iter_cleanup_reduce():
# Similar as above, but a complex reduction case that was previously
# missed (see gh-18810)/
# the following array is special in that it cananot be flattened:
# missed (see gh-18810).
# The following array is special in that it cannot be flattened:
arr = np.array([[None, 1], [-1, -1], [None, 2], [-1, -1]])[::2]
with pytest.raises(TypeError):
np.sum(arr)
Expand Down

0 comments on commit acf6648

Please sign in to comment.