To reproduce: 1. Go to http://build.webkit.org/builders/SnowLeopard%20Intel%20Leaks Each build says something like this: Failed leaks found for a total of 196,240 bytes! 1 unique leaks found! With ORWT, it used to say something like this: Failed 1400 total leaks found! Not having the total number of leaks printed makes it hard to see at a glance whether the number of leaks is increasing or decreasing. I suspect it also breaks Leaks Viewer's recent-leaky-builds list, which can be seen by going to <http://build.webkit.org/LeaksViewer/>.
We could get this number from re-parsing the files. But ORWT's design of having a single global variable as a counter cannot work. I'm happy to move --leaks processing back to ORWT until these two bugs are fixed. That's probably simplest given that I have to do some non-NRWT work this week.
I've reverted back to ORWT for the Leaks bot for now: http://trac.webkit.org/changeset/93046
Is this a blocking issue, or just a polish issue?
Well, it breaks one of our main tools for analyzing the results of the Leaks bot. So I guess you could consider it "blocking".
OK. So what specifically do you need to unbreak the tool? You need a total count of all leaks found? (which I can get by re-processing all of the *-leaks.txt files at the end). It seems that since LeaksViewer is already processing all the *-leaks.txt files, it would just count them up itself. The individual count per-file is printed out in the log as we go. We just don't print a total summary (except the summarized unique leaks) now.
(In reply to comment #5) > OK. So what specifically do you need to unbreak the tool? > > You need a total count of all leaks found? (which I can get by re-processing all of the *-leaks.txt files at the end). Right. > It seems that since LeaksViewer is already processing all the *-leaks.txt files, it would just count them up itself. That would dramatically increase the amount of time it takes for Leaks Viewer's "recent builds" list to appear.
<rdar://problem/10224916>
It looks like the leaks bot got switched back to ORWT. I'm investigating switching it to NRWT again today.
Created attachment 112741 [details] Patch
I believe this change works. I can't do a full system test (without fixing bug 71059 first), since it seems run-leaks doesn't work on lion?
Comment on attachment 112741 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112741&action=review > Tools/ChangeLog:12 > + * Scripts/webkitpy/layout_tests/port/leakdetector.py: > + * Scripts/webkitpy/layout_tests/port/leakdetector_unittest.py: > + * Scripts/webkitpy/layout_tests/port/mac.py: Can you add some function-level comments? That would make this patch a lot easier to review.
Created attachment 112750 [details] hopefully this is easier to review
Created attachment 112751 [details] include code to turn on NRWT for leaks bot
Comment on attachment 112750 [details] hopefully this is easier to review View in context: https://bugs.webkit.org/attachment.cgi?id=112750&action=review > Tools/Scripts/webkitpy/layout_tests/port/mac.py:142 > + total_bytes_string, unique_leaks = self._leak_detector.count_unique_leaks(leaks_files) It's a little weird that count_unique_leaks returns more than just a count of unique leaks. I don't have any great naming suggestions. > Tools/Scripts/webkitpy/layout_tests/port/mac.py:143 > + total_leaks = self._leak_detector.count_total_leaks(leaks_files) Would it be more efficient to count total and unique leaks in a single function (so you could pass through the leaks files just once)?
Comment on attachment 112751 [details] include code to turn on NRWT for leaks bot The run-webkit-tests changes are missing from your ChangeLog. But they should probably be done separately anyway. r=me on that bit. See my comments on the old patch for the Python changes.
(In reply to comment #14) > (From update of attachment 112750 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112750&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/mac.py:142 > > + total_bytes_string, unique_leaks = self._leak_detector.count_unique_leaks(leaks_files) > > It's a little weird that count_unique_leaks returns more than just a count of unique leaks. I don't have any great naming suggestions. Sure will make it count_total_bytes_and_unique_leaks. :) > > Tools/Scripts/webkitpy/layout_tests/port/mac.py:143 > > + total_leaks = self._leak_detector.count_total_leaks(leaks_files) > > Would it be more efficient to count total and unique leaks in a single function (so you could pass through the leaks files just once)? Doesn't really matter. Thats what I had originally, but I figured this might be slightly cleaner. The unique_leaks function has to call out to parse-malloc-history which has to read the files itself.
Comment on attachment 112750 [details] hopefully this is easier to review View in context: https://bugs.webkit.org/attachment.cgi?id=112750&action=review >>> Tools/Scripts/webkitpy/layout_tests/port/mac.py:143 >>> + total_leaks = self._leak_detector.count_total_leaks(leaks_files) >> >> Would it be more efficient to count total and unique leaks in a single function (so you could pass through the leaks files just once)? > > Doesn't really matter. Thats what I had originally, but I figured this might be slightly cleaner. The unique_leaks function has to call out to parse-malloc-history which has to read the files itself. Gotcha. The current separation seems fine then.
Created attachment 112754 [details] Patch for landing
This landed, svn was just hanging webkit-patch. :( http://trac.webkit.org/changeset/98639