browsermt / marian-dev

Fast Neural Machine Translation in C++ - development repository
https://marian-nmt.github.io
Other
20 stars 7 forks source link

Tensors allocated on cache does not appear deallocation capable #84

Open jerinphilip opened 2 years ago

jerinphilip commented 2 years ago

Assuming TensorAllocator is an attempt at implementing an equivalent of std::allocator.

Both tensors_ and cache_ in ExpressionGraph are TensorAllocators.

The cache_ allocates some tensors that are incapable of being deallocated from outside. cache_ is private to this class, and the missing free is a flaw(?).

https://github.com/browsermt/marian-dev/blob/844800efccba6e670250caac1735ca2c8c8e508e/src/graph/expression_graph.h#L57-L64

I could not find a cache_->free(), tensors_ on the other hand are capable of being freed manually through external calls.

https://github.com/browsermt/marian-dev/blob/844800efccba6e670250caac1735ca2c8c8e508e/src/graph/expression_graph.h#L70

jerinphilip commented 2 years ago
/marian-dev/src$ grep -R "setMemoize([^)]*)" --exclude-dir 3rd_party/
tensors/cpu/intgemm_interface.h:    setMemoize(false);
tensors/cpu/intgemm_interface.h:    setMemoize(false); // Enabling memoization leads to a massive memory leak. Instead use special "midterm" memory.
tensors/cpu/intgemm_interface.h:                       // Still, I don't understand why setMemoize(true) still leaks.
tensors/cpu/intgemm_interface.h:      setMemoize(false);
tensors/cpu/intgemm_interface.h:      setMemoize(false);
tensors/cpu/intgemm_interface.h:      setMemoize(false);
tensors/cpu/intgemm_interface.h:        setMemoize(false); // AFAIK dot is never called with the same matrices
tensors/cpu/intgemm_interface.h:        setMemoize(false); // AFAIK affine is never called with the same matrices
graph/node_operators.cpp:  setMemoize(graph->isInference());
graph/node.h:  virtual void setMemoize(bool memoize) override { memoize_ = memoize; };
graph/chainable.h:  virtual void setMemoize(bool) = 0;

Memoization appears to be set for nodes at inference.

jerinphilip commented 2 years ago

@XapaJIaMnu

Still, I don't understand why setMemoize(true) still leaks.