| Summary: | Enhancement: New Script to Format malloc_history output as tree | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||
| Component: | Tools / Tests | Assignee: | Michael Saboff <msaboff> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Enhancement | CC: | webkit.review.bot | ||||
| Priority: | P3 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Mac | ||||||
| OS: | OS X 10.5 | ||||||
| Attachments: |
|
||||||
|
Description
Michael Saboff
2011-03-15 08:35:48 PDT
Created attachment 85812 [details]
New script to process malloc_history output to tree format
Comment on attachment 85812 [details] New script to process malloc_history output to tree format View in context: https://bugs.webkit.org/attachment.cgi?id=85812&action=review This seems OK. It's written as a one-off w/o much re-use in mind, which is kinda the opposite of how I try to write python. :) I think this could use another round of cleanup to make this more hackable for others in the future. I'm glad you're adding this though. :) > Tools/Scripts/malloc-tree:60 > + I think pep8 says two spaces between globals, but I could be wrong. > Tools/Scripts/malloc-tree:102 > + def printNode(self, prefix = ' '): > + global hotspot > + global scaleSize > + global showBars Seems this could just take an options object instead of using globals. > Tools/Scripts/malloc-tree:137 > +def main(): I would have broken this up into smaller functions. > Tools/Scripts/malloc-tree:163 > + if hotspot: > + scaleSize = False > + else: > + scaleSize = True Seems long-winded. Comment on attachment 85812 [details]
New script to process malloc_history output to tree format
In general this is fine. We should land this and iterate.
Comment on attachment 85812 [details] New script to process malloc_history output to tree format Clearing flags on attachment: 85812 Committed r89195: <http://trac.webkit.org/changeset/89195> All reviewed patches have been landed. Closing bug. |