[webkit-reviews] review requested: [Bug 90080] ps based memory measurement for performance-tests : [Attachment 150415] proposed patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jul 2 06:49:03 PDT 2012
Zoltan Horvath <zoltan at webkit.org> has asked for review:
Bug 90080: ps based memory measurement for performance-tests
https://bugs.webkit.org/show_bug.cgi?id=90080
Attachment 150415: proposed patch
https://bugs.webkit.org/attachment.cgi?id=150415&action=review
------- Additional Comments from Zoltan Horvath <zoltan at webkit.org>
(In reply to comment #7)
> (From update of attachment 149962 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=149962&action=review
>
> > Tools/Scripts/webkitpy/layout_tests/port/driver.py:37
> > + def __init__(self, test_name, timeout, image_hash,
should_run_pixel_test, should_measure_memory=None, args=None):
>
> Nit: please use should_measure_memory=False for this.
Thanks Dirk, fixed!
(In reply to comment #11)
> View in context:
https://bugs.webkit.org/attachment.cgi?id=149962&action=review
>
> Just some observations:
>
> > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:569
> > + memlogger = MemoryLogger(self._server_process._pid)
>
> use 'pid()' instead of '_pid'.
Fixed.
> > Tools/Scripts/webkitpy/performance_tests/memorylogger.py:34
> > +import time
> > +import threading
> > +import subprocess
> > +import sys
> > +import re
>
> Should this be in alphabetic order?
Fixed.
> > Tools/Scripts/webkitpy/performance_tests/memorylogger.py:45
> > + while True and not self._stop:
>
> I think you can skip the 'True'.
>
> > Tools/Scripts/webkitpy/performance_tests/perftest.py:226
> > + if len(memory_values):
>
> No need for the len().
Fixed.
Thanks for the comments Mr.Python ;]
Ryosuke, it's your turn now.
More information about the webkit-reviews
mailing list