Report more information for memory profiling #61282

cloudhan · 2021-07-06T10:48:46Z

Report pointed memory size, total allocated memory, total reserved size all in one report.

ptr and alloc_size will be used for associating with op trace.
allocated_size, reserved_size will be used for memory trace.

facebook-github-bot · 2021-07-06T10:48:52Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/61282
📄 Preview docs built from this PR

💊 CI failures summary and remediations

As of commit d6130f3 (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.2-py3.6

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

cloudhan · 2021-07-06T10:49:23Z

Let's see what happens if we change the API function prototype.

cloudhan · 2021-07-07T11:58:32Z

It seems to be OK to change the API.

torch/csrc/autograd/profiler_kineto.cpp

gaoteng-git

Your comment at head of this PR should be changed. Suggest to explain why changing relative size to absolute size.
And the downstream profiler's processing on memory events will need to update when alloc_size is changed from relative size to absolute size.

torch/csrc/autograd/profiler_kineto.cpp

gdankel · 2021-07-08T21:44:46Z

Seems reasonable to me. cc @ilia-cher @ngimel

ngimel · 2021-07-08T22:13:44Z

c10/core/CPUAllocator.cpp

@@ -323,7 +324,7 @@ void ProfiledCPUMemoryReporter::Delete(void* ptr) {
  }
  if (profile_memory) {
    reportMemoryUsageToProfiler(
-        ptr, -nbytes, c10::Device(c10::DeviceType::CPU));
+        ptr, nbytes, allocated, 0, c10::Device(c10::DeviceType::CPU));


why did nbytes sign change?

whoops, a mistake when bringing it back. fixed

Hm, a mistake like this that's not caught by the tests is making me very nervous. Can you please add a test that would catch this?

Tests are added for CPU and CUDA. Some minor bug are found during testing.

I changed the cuda DeviceCachingAllocator to report Block wrapped ptr. It will be unwrapped by THCCachingAllocator and be wrapped it again as DataPtr. This will unify reported ptr semantic for CPU and CUDA allocator.

A bug is found due to free_block will change the passed in Block.

See the commit for details.

c10/core/Allocator.h

cloudhan · 2021-07-12T03:46:21Z

@ngimel I added cpu and cuda tests for reportMemoryUsage API. Hopefully it will catch API misuse in the future.

There is a lint error ask me to use angle bracket for include, I tried and it failed the compiling. How should I deal with it?

cloudhan · 2021-07-16T09:42:28Z

@gdankel Ping....

gdankel · 2021-07-16T09:53:56Z

c10/core/Allocator.h

  virtual void reportMemoryUsage(
      void* ptr,
      int64_t alloc_size,
+      int64_t allocated_size,


The naming is confusing. How about total_allocated and total_reserved?

New names will be much better. Will change.

record both the ptr size, total allocated size and total reserved size

… DefaultCPUAllocator

…rmation before the change

…ated and total_reserved

gdankel

LGTM!

facebook-github-bot · 2021-07-20T17:17:54Z

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

facebook-github-bot · 2021-08-04T22:04:28Z

@chaekit merged this pull request in 8bbcef5.

cloudhan requested review from albanD and soulitzer as code owners July 6, 2021 10:48

facebook-github-bot added the cla signed label Jul 6, 2021

pytorchbot added the open source label Jul 6, 2021

cloudhan force-pushed the memory_profile branch from 11d0fea to 0a37749 Compare July 7, 2021 03:23

guotuofeng reviewed Jul 8, 2021

View reviewed changes

torch/csrc/autograd/profiler_kineto.cpp Outdated Show resolved Hide resolved

gaoteng-git suggested changes Jul 8, 2021

View reviewed changes

torch/csrc/autograd/profiler_kineto.cpp Show resolved Hide resolved

cloudhan force-pushed the memory_profile branch from 4a5d687 to 71cb0d7 Compare July 8, 2021 08:34

iramazanli added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 8, 2021

ngimel reviewed Jul 8, 2021

View reviewed changes

cloudhan force-pushed the memory_profile branch from 71cb0d7 to fce5446 Compare July 9, 2021 02:29

cloudhan changed the title ~~[DRAFT] memory profile change~~ Report more information for memory profiling Jul 9, 2021

cloudhan force-pushed the memory_profile branch from 7a38f73 to 0b0a2ef Compare July 9, 2021 11:59

albanD removed their request for review July 9, 2021 16:02

cloudhan force-pushed the memory_profile branch from 4e365c8 to 73b6424 Compare July 12, 2021 03:38

soulitzer removed their request for review July 15, 2021 21:13

gdankel reviewed Jul 20, 2021

View reviewed changes

cloudhan added 7 commits July 20, 2021 13:21

change memory report signature

f4d9560

record both the ptr size, total allocated size and total reserved size

fix issue and update comment

acba1cd

fix formating

b823946

report the ptr that is wrapped DataPtr, make the semantic the same as…

4746484

… DefaultCPUAllocator

free_block might change the underlaying Block, store the related info…

8328b24

…rmation before the change

checkin memory reporter tests

78d8848

type, grammar and format

a9dac18

cloudhan added 3 commits July 20, 2021 13:21

use int for ptr output

7ca3eaf

add one assert for Addr

e336532

better naming, rename allocated_size and reserved_size to total_alloc…

d94c46e

…ated and total_reserved

cloudhan force-pushed the memory_profile branch from fff51de to d94c46e Compare July 20, 2021 05:21

gdankel approved these changes Jul 20, 2021

View reviewed changes

cloudhan added 2 commits July 28, 2021 19:51

Merge branch 'master' into memory_profile

60fa39f

minor update and format

c0398a0

cloudhan requested a review from gdankel July 28, 2021 16:54

mute linter

d6130f3

facebook-github-bot closed this in 8bbcef5 Aug 4, 2021

facebook-github-bot added the Merged label Aug 4, 2021

cloudhan deleted the memory_profile branch September 26, 2021 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report more information for memory profiling #61282

Report more information for memory profiling #61282

cloudhan commented Jul 6, 2021 •

edited

Loading

facebook-github-bot commented Jul 6, 2021 •

edited

Loading

cloudhan commented Jul 6, 2021

cloudhan commented Jul 7, 2021

gaoteng-git left a comment

gdankel commented Jul 8, 2021

ngimel Jul 8, 2021

cloudhan Jul 9, 2021

ngimel Jul 9, 2021

cloudhan Jul 9, 2021

cloudhan commented Jul 12, 2021

cloudhan commented Jul 16, 2021

gdankel Jul 16, 2021

cloudhan Jul 20, 2021

gdankel left a comment

facebook-github-bot commented Jul 20, 2021

facebook-github-bot commented Aug 4, 2021

Report more information for memory profiling #61282

Report more information for memory profiling #61282

Conversation

cloudhan commented Jul 6, 2021 • edited Loading

facebook-github-bot commented Jul 6, 2021 • edited Loading

🔗 Helpful links

💊 CI failures summary and remediations

ci.pytorch.org: 1 failed

cloudhan commented Jul 6, 2021

cloudhan commented Jul 7, 2021

gaoteng-git left a comment

Choose a reason for hiding this comment

gdankel commented Jul 8, 2021

ngimel Jul 8, 2021

Choose a reason for hiding this comment

cloudhan Jul 9, 2021

Choose a reason for hiding this comment

ngimel Jul 9, 2021

Choose a reason for hiding this comment

cloudhan Jul 9, 2021

Choose a reason for hiding this comment

cloudhan commented Jul 12, 2021

cloudhan commented Jul 16, 2021

gdankel Jul 16, 2021

Choose a reason for hiding this comment

cloudhan Jul 20, 2021

Choose a reason for hiding this comment

gdankel left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jul 20, 2021

facebook-github-bot commented Aug 4, 2021

cloudhan commented Jul 6, 2021 •

edited

Loading

facebook-github-bot commented Jul 6, 2021 •

edited

Loading