Skip to content

Commit

Permalink
Update on "Remove DataPtr extractor from CUDAFuture"
Browse files Browse the repository at this point in the history
The CUDAFuture class needs to inspect the values it contains in order to extract its tensors (in fact, the DataPtrs backing those). These are needed first to determine what CUDA devices back those tensors, so that an event for each such device can be recorded; and later to record these DataPtrs with the CUDA caching allocator if they are used in other streams.

This became complicated when Python was added to the mix, because to inspect a Python object we need to acquire the GIL, but we couldn't do so from code that was supposed to also work in C++-only mode. The solution was for users to provide a custom way to extract DataPtrs, so that the PythonFutureWrapper could install such a custom Python-aware one. This was the DataPtr extractor.

In #48502 a different suggestion was proposed. At its root, it consists in adding support for IValues of type PyObject to the visit() and getSubValues() methods. In order to deal with the GIL, we do this through a virtual method: PyObjectHolder, which is the base class, is available also in C++-only mode, and thus defines this method but leaves it unimplemented; ConcretePyObjectHolder, which is the subclass, is only included in Python mode, and thus it can implement that method, acquire the GIL, and do what it's supposed to.

In my opinion, this approach is just brilliant! Thank @wanchaol for proposing it! It hides the complexity of dealing with Python inside getSubValues(), where it can be done properly, thus simplifying enormously the CUDAFuture and the PythonFutureWrapper classes.

Differential Revision: [D25334355](https://our.internmc.facebook.com/intern/diff/D25334355/)

[ghstack-poisoned]
  • Loading branch information
lw committed Dec 16, 2020
2 parents bb5cc76 + ca64b25 commit 96e6860
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion aten/src/ATen/core/ivalue_inl.h
Expand Up @@ -7,7 +7,6 @@
#include <ATen/core/List.h>
#include <ATen/core/functional.h>
#include <ATen/core/interned_strings.h>
#include <ATen/core/jit_type.h>
#include <ATen/core/qualified_name.h>
#include <ATen/core/rref_interface.h>
#include <c10/core/Scalar.h>
Expand Down

0 comments on commit 96e6860

Please sign in to comment.