Skip to content

Fix shared_ptr refcycle in graph executor - #10222

Closed
zou3519 wants to merge 1 commit into
pytorch:masterfrom
zou3519:pytorch-refcycle
Closed

zou3519 wants to merge 1 commit into
pytorch:masterfrom
zou3519:pytorch-refcycle

Conversation

@zou3519

@zou3519 zou3519 commented Aug 3, 2018

Copy link
Copy Markdown
Contributor

Fixes #10032

When capturing an output, GraphExecutorAutogradFunction creates
SavedVariable with is_output=False and owns it:
https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/graph_executor.cpp#L87

Constructing SavedVariable with is_output=False makes it own a copy of
the shared_ptr, which causes a reference
cycle:

grad_fn_ = variable.grad_fn();

The solution in this PR is to construct the SavedVariable with
is_output=True if the captured value is an output.

Test Plan

Turn on cuda memory checking for JitTestCase. If the test's name
includes "cuda" or "gpu" in it, the cuda memory checking test happens.

cc @zdevito

Fixes pytorch#10032

When capturing an output, GraphExecutorAutogradFunction creates
SavedVariable with is_output=False and owns it:
https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/graph_executor.cpp#L87

Constructing SavedVariable with is_output=False makes it own a copy of
the shared_ptr<GraphExecutorAutogradFunction>, which causes a reference
cycle:
https://github.com/pytorch/pytorch/blob/6456b944fd3dfe1b7db830b27afd44b15ba5a6e9/torch/csrc/autograd/saved_variable.cpp#L27

The solution in this PR is to construct the SavedVariable with
is_output=True if the captured value is an output.

Test Plan

Turn on cuda memory checking for JitTestCase. If the test's name
includes "cuda" or "gpu" in it, the cuda memory checking test happens.
@zou3519 zou3519 added the oncall: jit Add this issue/PR to JIT oncall triage queue label Aug 3, 2018

@facebook-github-bot facebook-github-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zou3519 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary:
Fixes pytorch#10032

When capturing an output, GraphExecutorAutogradFunction creates
SavedVariable with is_output=False and owns it:
https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/graph_executor.cpp#L87

Constructing SavedVariable with is_output=False makes it own a copy of
the shared_ptr<GraphExecutorAutogradFunction>, which causes a reference
cycle:
https://github.com/pytorch/pytorch/blob/6456b944fd3dfe1b7db830b27afd44b15ba5a6e9/torch/csrc/autograd/saved_variable.cpp#L27

The solution in this PR is to construct the SavedVariable with
is_output=True if the captured value is an output.

Test Plan

Turn on cuda memory checking for JitTestCase. If the test's name
includes "cuda" or "gpu" in it, the cuda memory checking test happens.

cc zdevito
Pull Request resolved: pytorch#10222

Reviewed By: ezyang

Differential Revision: D9162995

Pulled By: zou3519

fbshipit-source-id: aeace85a09160c7a7e79cf35f6ac61eac87cbf66
@ezyang ezyang added the merged label Jun 26, 2019
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Fixes pytorch#10032

When capturing an output, GraphExecutorAutogradFunction creates
SavedVariable with is_output=False and owns it:
https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/graph_executor.cpp#L87

Constructing SavedVariable with is_output=False makes it own a copy of
the shared_ptr<GraphExecutorAutogradFunction>, which causes a reference
cycle:
https://github.com/pytorch/pytorch/blob/75494dc267640166b7b75891f5bbc761e0ceec23/torch/csrc/autograd/saved_variable.cpp#L27

The solution in this PR is to construct the SavedVariable with
is_output=True if the captured value is an output.

Test Plan

Turn on cuda memory checking for JitTestCase. If the test's name
includes "cuda" or "gpu" in it, the cuda memory checking test happens.

cc zdevito
Pull Request resolved: pytorch#10222

Reviewed By: ezyang

Differential Revision: D9162995

Pulled By: zou3519

fbshipit-source-id: aeace85a09160c7a7e79cf35f6ac61eac87cbf66
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3x regression in JIT LSTM speeds

5 participants