Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
148133003:
Better grouping of data in Deep Memory Profiler (Closed)
|
Created:
6 years, 11 months ago by Daniel Bratell Modified:
6 years, 10 months ago Reviewers:
Dai Mikurube (NOT FULLTIME) CC:
chromium-reviews, dmikurube+memory_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionBetter grouping of data in Deep Memory Profiler
In the linux browser profile, put string data under their
own sections, and teach the system about style and css memory.
R=
BUG=
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248996
Patch Set 1 #
Total comments: 1
Patch Set 2 : Skipped the logging changes. #
Messages
Total messages: 20 (0 generated)
Just some random changes I had in my tree since last year and forgot to upload.
Thanks for working on it. The change in the policy looks good, but the logging was actually intentional. https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... File tools/deep_memory_profiler/dmprof.py (right): https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... tools/deep_memory_profiler/dmprof.py:64: logging.basicConfig() No. Here, I name LOGGER as 'dmprof' at line 17. The LOGGER is used in dmprof-related Python code. I use the original namespace for the LOGGER since dmprof can be used as a library from other code.
On 2014/01/28 01:49:29, Dai Mikurube wrote: > Thanks for working on it. The change in the policy looks good, but the logging > was actually intentional. > > https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... > File tools/deep_memory_profiler/dmprof.py (right): > > https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... > tools/deep_memory_profiler/dmprof.py:64: logging.basicConfig() > No. Here, I name LOGGER as 'dmprof' at line 17. The LOGGER is used in > dmprof-related Python code. > > I use the original namespace for the LOGGER since dmprof can be used as a > library from other code. Ah, then I could just move it to a __name__ == "__main__" block.
On 2014/01/28 09:09:43, Daniel Bratell wrote: > On 2014/01/28 01:49:29, Dai Mikurube wrote: > > Thanks for working on it. The change in the policy looks good, but the logging > > was actually intentional. > > > > > https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... > > File tools/deep_memory_profiler/dmprof.py (right): > > > > > https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... > > tools/deep_memory_profiler/dmprof.py:64: logging.basicConfig() > > No. Here, I name LOGGER as 'dmprof' at line 17. The LOGGER is used in > > dmprof-related Python code. > > > > I use the original namespace for the LOGGER since dmprof can be used as a > > library from other code. > > Ah, then I could just move it to a __name__ == "__main__" block. It already is inside __name__ == "__main__" so this is not executed either before or after the change if dmprof is used as a library.
On 2014/01/28 10:05:02, Daniel Bratell wrote: > On 2014/01/28 09:09:43, Daniel Bratell wrote: > > On 2014/01/28 01:49:29, Dai Mikurube wrote: > > > Thanks for working on it. The change in the policy looks good, but the > logging > > > was actually intentional. > > > > > > > > > https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... > > > File tools/deep_memory_profiler/dmprof.py (right): > > > > > > > > > https://codereview.chromium.org/148133003/diff/1/tools/deep_memory_profiler/d... > > > tools/deep_memory_profiler/dmprof.py:64: logging.basicConfig() > > > No. Here, I name LOGGER as 'dmprof' at line 17. The LOGGER is used in > > > dmprof-related Python code. > > > > > > I use the original namespace for the LOGGER since dmprof can be used as a > > > library from other code. > > > > Ah, then I could just move it to a __name__ == "__main__" block. > > It already is inside __name__ == "__main__" so this is not executed either > before or after the change if dmprof is used as a library. No one under deep_memory_profiler/ logs via the root logger (logging.*). All logs are via LOGGER = logging.getLogger('dmprof'). So, logging.basicConfig() has no meaning here.
On 2014/01/28 10:37:21, Dai Mikurube wrote:
> No one under deep_memory_profiler/ logs via the root logger (logging.*). All
> logs are via LOGGER = logging.getLogger('dmprof'). So, logging.basicConfig()
has
> no meaning here.
Dropped the logging part. Just the changes to the json file now. How does it
look now?
lgtm. Thanks!
The CQ bit was checked by bratell@opera.com
The CQ bit was unchecked by bratell@opera.com
The CQ bit was checked by bratell@opera.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/bratell@opera.com/148133003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on android_aosp for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...
The CQ bit was checked by bratell@opera.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/bratell@opera.com/148133003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on android_aosp for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...
The CQ bit was checked by bratell@opera.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/bratell@opera.com/148133003/40001
Message was sent while issue was closed.
Change committed as 248996 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
