Fix infer np scalar dtype mem leak #24267
Conversation
pytorchbot
added
module: dataloader
|
If we aren't confident in the test, we can just not add the test. |
torch/csrc/utils/tensor_new.cpp
Outdated
| @@ -166,7 +166,10 @@ ScalarType infer_scalar_type(PyObject *obj) { | |||
| return numpy_dtype_to_aten(PyArray_TYPE((PyArrayObject*)obj)); | |||
| } | |||
| if (PyArray_CheckScalar(obj)) { | |||
| return numpy_dtype_to_aten(PyArray_TYPE((PyArrayObject*)(PyArray_FromScalar(obj, nullptr)))); | |||
| auto arr = PyArray_FromScalar(obj, nullptr); | |||
There was a problem hiding this comment.
can we just use THPObjectPtr or whatever here so we don't have to do manually memory management?
|
@gchanan This is now updated to use |
ssnl
force-pushed
the
infer_np_scalar_mem_leak
branch
from
c3f31cf
to
49cba7c
Compare
ssnl
force-pushed
the
infer_np_scalar_mem_leak
branch
from
49cba7c
to
cc5d4fb
Compare
|
@gchanan pinging for review :) |
use THPObjectPtr
ssnl
force-pushed
the
infer_np_scalar_mem_leak
branch
from
cc5d4fb
to
8c60852
Compare
|
I removed the test. |
There was a problem hiding this comment.
@gchanan has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
I'll land it -- are you planning to add the test in a follow up? |
|
@gchanan It'll be difficult to add a test without examine CPU memory usage, which could be flaky in Python. I had one such test in an earlier version of this PR, and CI passed fine. But I was worried about the potential flakiness and removed it. In the end, it is unlikely that some future patch breaks this. |
Fixes #24200 . I'm a bit worried that the test might be flaky...