In WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium.py, there's a 3 second timeout when closing the DRT process pipe where we wait for the DRT process to shutdown. When running DRT in valgrind, this takes longer than 3 seconds and we kill the process before getting memory leak information. Maybe we can somehow share the --time-out-ms flag to increase this value? I'm not sure if there's some other way to detect that we're running under valgrind to increase this timeout. Maybe thestig knows.
I mean, we could also add back the TOOL_WRAPPER environment if valgrind really needs it. :) But yes, it seems like a reasonable heuristic that we could just double that value when time_out_ms is larger than default.
I already fixed the script on the Chromium side to use --wrapper. Just need to fix this bit here. Can we just use --time-out-ms as the timer value here? The script on the Chromium side is already passing --time-out-ms=200000. Doubling the timeout value is not enough. I set to 180 seconds locally just to be on the safe side , but I can see what's a good, lower value if needed.
Is this generally true when trying to use valgrind with run-webkit-tests on linux/mac? Will other ports want something like this?
I don't know if other ports run Valgrind with --time-out-ms=$large_number, but I imagine they would have to given Valgrind's slowness.
> I'm not sure if there's some other way to detect > that we're running under valgrind to increase this timeout. Use RUNNING_ON_VALGRIND ? http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/valgrind/valgrind.h?revision=72632&view=markup
Also, you can add RunningOnValgrind() from http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/dynamic_annotations/dynamic_annotations.c?revision=84975&view=markup to JavaScriptCore/wtf/DynamicAnnotations.{h,cpp}
(In reply to comment #6) > Also, you can add RunningOnValgrind() from http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/dynamic_annotations/dynamic_annotations.c?revision=84975&view=markup > to JavaScriptCore/wtf/DynamicAnnotations.{h,cpp} Unfortunately I don't see how we could access that from the python code. Do we have any idea how long it does take for DRT to shut down, so we know how long we should be scaling up to? It seems reasonable to scale up the 3 second timeout based on how much --time-out-ms is scaled up over the default.
If the current 3 second timeout is meant for a machine like a Z600, then 15 seconds should be fine for Valgrind. (I ran a bunch of tests and see ~8 seconds nromally and doubled it to be safe.)
Created attachment 108390 [details] [chromium] Make the layout test script's kill timeout proportional to --time-out-ms Ping? Would something like this work?
Comment on attachment 108390 [details] [chromium] Make the layout test script's kill timeout proportional to --time-out-ms Hi, sorry for the delay in getting back to you (I've been backlogged with other things). This looks fine; do you need me to CQ+ it or land it for you?
Comment on attachment 108390 [details] [chromium] Make the layout test script's kill timeout proportional to --time-out-ms Needs a changelog :)
(In reply to comment #11) > (From update of attachment 108390 [details]) > Needs a changelog :) Whoops :(
Created attachment 108411 [details] patch with changelog Ha, my Chromium checkout checked out Tools/Scripts, but not Tools, thus I couldn't find the ChangeLog and when *shrug*.
Created attachment 108520 [details] Patch for landing
(In reply to comment #14) > Created an attachment (id=108520) [details] > Patch for landing Fixed the svn root and added the bug link to the changelog.
Comment on attachment 108520 [details] Patch for landing Clearing flags on attachment: 108520 Committed r95875: <http://trac.webkit.org/changeset/95875>
All reviewed patches have been landed. Closing bug.