Bug 168580

Summary: JSC should have a marathon test mode
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: Tools / TestsAssignee: Filip Pizlo <fpizlo>
Status: NEW    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, dbates, keith_miller, lforschler, mark.lam, msaboff, ossy, saam, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 168585, 168687, 168697    
Bug Blocks:    
Attachments:
Description Flags
the patch ysuzuki: review+

Filip Pizlo
Reported 2017-02-19 17:49:21 PST
JSC tests currently run in a process per test. But running all tests in one process will help catch more GC bugs because it creates a very polluted heap. We should run a new test mode, called marathon, that runs a bunch of tests in a single process. It should run concurrently to the other tests.
Attachments
the patch (13.67 KB, patch)
2017-02-19 17:54 PST, Filip Pizlo
ysuzuki: review+
Filip Pizlo
Comment 1 2017-02-19 17:54:27 PST
Created attachment 302103 [details] the patch It'll be hard to land this because it finds crashes.
WebKit Commit Bot
Comment 2 2017-02-19 17:55:49 PST
Attachment 302103 [details] did not pass style-queue: ERROR: Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] ERROR: Source/WTF/wtf/NumberOfCores.cpp:53: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 3 2017-02-20 17:21:07 PST
Disappointed to see this was not you implementing Bungie's Marathon (https://en.wikipedia.org/wiki/Marathon_(video_game)) in the jsc shell.
Yusuke Suzuki
Comment 4 2017-03-05 22:09:28 PST
Comment on attachment 302103 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=302103&action=review r=me > JSTests/stress/marathon.js:14 > + string = "quit = function() { throw new Error(\"---CALLED-QUIT---\"); }\n" + string; I recommend to add the stub for `abort()` too. > Source/JavaScriptCore/jsc.cpp:1372 > + addFunction(vm, "callInDirectory", functionCallInDirectory, 2); Let's put `#if !OS(WINDOWS)` things to fix build failure in Windows. > Source/JavaScriptCore/jsc.cpp:2665 > + int previousDirectoryFD = open(".", O_RDONLY); Let's add necessary headers to fix GTK build failures (maybe, `sys/stat.h` and `fcntl.h`.) > Source/JavaScriptCore/jsc.cpp:2667 > + int myErrno = errno; `myErrno` variable can be dropped. > Source/JavaScriptCore/jsc.cpp:2668 > + return JSValue::encode(throwException(exec, scope, createError(exec, toString("Cannot open current directory: ", strerror(myErrno))))); How about using `throwVMError`? > Source/JavaScriptCore/jsc.cpp:2673 > + int myErrno = errno; Ditto. > Source/JavaScriptCore/jsc.cpp:2674 > + return JSValue::encode(throwException(exec, scope, createError(exec, toString("Cannot change directory to ", fileName, ": ", strerror(myErrno))))); Ditto. > Source/JavaScriptCore/jsc.cpp:2682 > + return JSValue::encode(throwException(exec, scope, createError(exec, ASCIILiteral("Expected callback")))); Use throwVMError.
Note You need to log in before you can comment on or make changes to this bug.