Bug 163804 – JavaScriptCore tests should have a mode to use a shared jsc process

• Component: Tools / Tests
• Status: RESOLVED WONTFIX

[Christopher Reid]

This problem is a bit specific to us, we have a huge ~6s overhead for launching processes. jsc-stress-tests takes over 24 hours to run due to each test launching its own process.
We would like a way to share a jsc process between tests.

The approach we were thinking of implementing was to use stdio on a shared process for tests. Because the test runner uses shell scripts and ruby, it doesn't seem possible on all platforms to send input to that process across multiple shell scripts.
We would likely need a way to use only ruby to run tests for this to work.

[Christopher Reid]

An alternative solution would be to write a client and server wrapping jsc. This would avoid having to re-write the test scripts with ruby but it's adding a potentially unwanted layer of complexity.

Are there any preferred approaches to this?

[Geoffrey Garen]

DumpRenderTree accepts a series of file names through stdio and runs them sequentially. I think it would be fine to add a command-line option for that to jsc.cpp.

I'm not sure how to solve your broader problem of not being able to launch processes for the scripted testing harness. Our test harness is pretty strongly designed around being able to launch processes.

[Christopher Reid]

Created attachment 295764 [details]
Patch

The batch mode param is right now a lot more coupled in to the jsc-tests than I would like. I was mainly wanting some initial feedback on what would like to be seen.

[WebKit Commit Bot]

Attachment 295764 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jsc.cpp:2856:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:2867:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:2867:  Missing spaces around <  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/jsc.cpp:2891:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:2891:  Missing spaces around <  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/jsc.cpp:3180:  Extra space before )  [whitespace/parens] [2]
Total errors found: 6 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 295764 [details]
Patch

This is a very invasive change to the test harness, and most JSC developers have platforms that have fast process start.  We rely on the fact that tests are isolated in a lot of subtle ways, including relying on shell scripts that wrap each test.  I don't want to lose this invariant in the test harness, because it will make all of our other test hacking harder.

[Brady Eidson]

Comment on attachment 295764 [details]
Patch

Patches that have been up for review since 2016 are almost certainly too stale to be relevant to trunk in their current form.

If this patch is still important please rebase it and post it for review again.

[Christopher Reid]

Closing this as it is no longer relevant.