Log Message
The default testing mode should not involve disabling the FTL JIT https://bugs.webkit.org/show_bug.cgi?id=159929
Rubber stamped by Mark Lam and Saam Barati.
Source/_javascript_Core:
Use the new powers to make some tests run only in the default configuration (i.e. FTL,
concurrent JIT).
* tests/mozilla/mozilla-tests.yaml:
Tools:
It used to be the case that most actively maintained ports did not have the FTL JIT enabled.
Heck, for most of the FTL's initial development, it wasn't enabled anywhere. So, testing the
FTL was not the default. You had to enable it with an option.
For some reason we have kept this arrangement even though the FTL JIT is now the default on
all of the major ports. This has become a serious pain. For example, it's useful to be able
to say that a test should only run in the default config that is representative of what a
normal user would see if they ran JSC. Clearly, this would be a config that does not
explicitly disable the FTL JIT on the command line. However, if you try to specify this then
your test won't run at all if the --ftl-jit option is not passed. That's dangerous!
So, this change gets rid of all of this logic. I think it's better to get rid of it then to
try to fix it, because:
- I don't know what the fix would look like. Presumably it would ensure that ports that don't
have the FTL enabled never run any tests that explicitly disable the FTL, since that
doesn't do anything. The code is not really structured to allow this.
- It benefits a minority of clients. Three build bots run tests in a config that has the FTL
disabled in testing. On the other hand there are already build bots that do FTL tests
despite having the FTL disabled by virtue of being a 32-bit platform. So, maybe rather than
preserving this broken feature, we should create something that (a) acknowledges the fact
that the FTL is the default on those platforms that support it and (b) avoids running
no-ftl tests on precisely those platforms that don't have FTL.
- To the extent that some bots benefited from disabling FTL tests, they were doing it by
relying on a feature that was never meant to stick around. The FTL JIT is meant to be the
default configuration. Disabling the FTL JIT is the non-default. So, we shouldn't be
pretending that the FTL JIT is not the default just because some bots used that as an
optimization.
This change allows me to speed up some debug tests and paint some bots green.
* Scripts/run-_javascript_core-tests:
(runJSCStressTests):
* Scripts/run-jsc-stress-tests:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (203439 => 203440)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-20 01:29:25 UTC (rev 203439)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-20 01:45:25 UTC (rev 203440)
@@ -1,3 +1,15 @@
+2016-07-19 Filip Pizlo <[email protected]>
+
+ The default testing mode should not involve disabling the FTL JIT
+ https://bugs.webkit.org/show_bug.cgi?id=159929
+
+ Rubber stamped by Mark Lam and Saam Barati.
+
+ Use the new powers to make some tests run only in the default configuration (i.e. FTL,
+ concurrent JIT).
+
+ * tests/mozilla/mozilla-tests.yaml:
+
2016-07-19 Keith Miller <[email protected]>
Test262 should have a file with the revision and url
Modified: trunk/Source/_javascript_Core/tests/mozilla/mozilla-tests.yaml (203439 => 203440)
--- trunk/Source/_javascript_Core/tests/mozilla/mozilla-tests.yaml 2016-07-20 01:29:25 UTC (rev 203439)
+++ trunk/Source/_javascript_Core/tests/mozilla/mozilla-tests.yaml 2016-07-20 01:45:25 UTC (rev 203440)
@@ -592,13 +592,13 @@
- path: ecma/FunctionObjects/15.3.1.1-2.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.1.1-3.js
- cmd: defaultRunMozillaTest :normal, "../shell.js"
+ cmd: runMozillaTestDefaultFTL :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.2.1-1.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.2.1-2.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.2.1-3.js
- cmd: defaultRunMozillaTest :normal, "../shell.js"
+ cmd: runMozillaTestDefaultFTL :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.3.1-1.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.3.1-2.js
@@ -616,7 +616,7 @@
- path: ecma/FunctionObjects/15.3.4.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.5-1.js
- cmd: defaultRunMozillaTest :normal, "../shell.js"
+ cmd: runMozillaTestDefaultFTL :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.5-2.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma/FunctionObjects/15.3.5.1.js
Modified: trunk/Tools/ChangeLog (203439 => 203440)
--- trunk/Tools/ChangeLog 2016-07-20 01:29:25 UTC (rev 203439)
+++ trunk/Tools/ChangeLog 2016-07-20 01:45:25 UTC (rev 203440)
@@ -1,3 +1,47 @@
+2016-07-19 Filip Pizlo <[email protected]>
+
+ The default testing mode should not involve disabling the FTL JIT
+ https://bugs.webkit.org/show_bug.cgi?id=159929
+
+ Rubber stamped by Mark Lam and Saam Barati.
+
+ It used to be the case that most actively maintained ports did not have the FTL JIT enabled.
+ Heck, for most of the FTL's initial development, it wasn't enabled anywhere. So, testing the
+ FTL was not the default. You had to enable it with an option.
+
+ For some reason we have kept this arrangement even though the FTL JIT is now the default on
+ all of the major ports. This has become a serious pain. For example, it's useful to be able
+ to say that a test should only run in the default config that is representative of what a
+ normal user would see if they ran JSC. Clearly, this would be a config that does not
+ explicitly disable the FTL JIT on the command line. However, if you try to specify this then
+ your test won't run at all if the --ftl-jit option is not passed. That's dangerous!
+
+ So, this change gets rid of all of this logic. I think it's better to get rid of it then to
+ try to fix it, because:
+
+ - I don't know what the fix would look like. Presumably it would ensure that ports that don't
+ have the FTL enabled never run any tests that explicitly disable the FTL, since that
+ doesn't do anything. The code is not really structured to allow this.
+
+ - It benefits a minority of clients. Three build bots run tests in a config that has the FTL
+ disabled in testing. On the other hand there are already build bots that do FTL tests
+ despite having the FTL disabled by virtue of being a 32-bit platform. So, maybe rather than
+ preserving this broken feature, we should create something that (a) acknowledges the fact
+ that the FTL is the default on those platforms that support it and (b) avoids running
+ no-ftl tests on precisely those platforms that don't have FTL.
+
+ - To the extent that some bots benefited from disabling FTL tests, they were doing it by
+ relying on a feature that was never meant to stick around. The FTL JIT is meant to be the
+ default configuration. Disabling the FTL JIT is the non-default. So, we shouldn't be
+ pretending that the FTL JIT is not the default just because some bots used that as an
+ optimization.
+
+ This change allows me to speed up some debug tests and paint some bots green.
+
+ * Scripts/run-_javascript_core-tests:
+ (runJSCStressTests):
+ * Scripts/run-jsc-stress-tests:
+
2016-07-19 Alex Christensen <[email protected]>
Fix API test after r203426.
Modified: trunk/Tools/Scripts/run-_javascript_core-tests (203439 => 203440)
--- trunk/Tools/Scripts/run-_javascript_core-tests 2016-07-20 01:29:25 UTC (rev 203439)
+++ trunk/Tools/Scripts/run-_javascript_core-tests 2016-07-20 01:45:25 UTC (rev 203440)
@@ -65,7 +65,6 @@
my $gmallocPath = undef;
my $gmallocDefaultPath = "/usr/lib/libgmalloc.dylib";
-my $enableFTL = isAppleMacWebKit() || isX86_64() && (isGtk() || isEfl());
my $createTarball = 0;
my $remoteHost = 0;
my $failFast = 1;
@@ -148,7 +147,6 @@
'root=s' => \$root,
'extra-tests=s' => \$extraTests,
'build!' => \$buildJSC,
- 'ftl-jit!' => \$enableFTL,
'testapi!' => \$runTestAPI,
'jsc-stress!' => \$runJSCStress,
'jit-stress-tests!' => \$runJITStressTests,
@@ -171,11 +169,6 @@
# Assume any arguments left over from GetOptions are assumed to be build arguments
my @buildArgs = @ARGV;
-# The --ftl-jit argument gets passed as a build argument.
-if ($enableFTL) {
- push(@buildArgs, '--ftl-jit');
-}
-
if ($showHelp) {
print STDERR $usage;
exit 1;
@@ -210,7 +203,7 @@
my $productDir = jscProductDir();
$ENV{DYLD_FRAMEWORK_PATH} = $productDir;
-$ENV{JSCTEST_timeout} = 60 unless $ENV{JSCTEST_timeout}; # Set a 60 second timeout on all jsc tests (if environment variable not defined already).
+$ENV{JSCTEST_timeout} = 120 unless $ENV{JSCTEST_timeout}; # Set a 120 second timeout on all jsc tests (if environment variable not defined already).
$ENV{TZ}="US/Pacific"; # Some tests fail if the time zone is not set to US/Pacific (<https://webkit.org/b/136363>)
setPathForRunningWebKitApp(\%ENV) if isCygwin();
@@ -316,10 +309,6 @@
if (!$runJITStressTests) {
push(@jscStressDriverCmd, "--no-jit");
- } else {
- if ($enableFTL) {
- push(@jscStressDriverCmd, "--ftl-jit");
- }
}
if ($createTarball) {
push(@jscStressDriverCmd, "--tarball");
Modified: trunk/Tools/Scripts/run-jsc-stress-tests (203439 => 203440)
--- trunk/Tools/Scripts/run-jsc-stress-tests 2016-07-20 01:29:25 UTC (rev 203439)
+++ trunk/Tools/Scripts/run-jsc-stress-tests 2016-07-20 01:45:25 UTC (rev 203440)
@@ -97,7 +97,6 @@
$jscPath = nil
$doNotMessWithVMPath = false
-$enableFTL = false
$jitTests = true
$memoryLimited = false
$outputDir = Pathname.new("results")
@@ -124,7 +123,6 @@
puts "--jsc (-j) Path to _javascript_Core build product. This option is required."
puts "--no-copy Do not copy the _javascript_Core build product before testing."
puts " --jsc specifies an already present _javascript_Core to test."
- puts "--ftl-jit Indicate that we have the FTL JIT."
puts "--memory-limited Indicate that we are targeting the test for a memory limited device."
puts " Skip tests tagged with //@large-heap"
puts "--no-jit Do not run JIT specific tests."
@@ -158,7 +156,6 @@
GetoptLong.new(['--help', '-h', GetoptLong::NO_ARGUMENT],
['--jsc', '-j', GetoptLong::REQUIRED_ARGUMENT],
['--no-copy', GetoptLong::NO_ARGUMENT],
- ['--ftl-jit', GetoptLong::NO_ARGUMENT],
['--memory-limited', GetoptLong::NO_ARGUMENT],
['--no-jit', GetoptLong::NO_ARGUMENT],
['--output-dir', '-o', GetoptLong::REQUIRED_ARGUMENT],
@@ -188,8 +185,6 @@
$doNotMessWithVMPath = true
when '--output-dir'
$outputDir = Pathname.new(arg)
- when '--ftl-jit'
- $enableFTL = true
when '--memory-limited'
$memoryLimited = true
when '--no-jit'
@@ -276,11 +271,6 @@
puts "Using the following jsc path: #{jscArg}"
end
-if $enableFTL and !$jitTests
- $stderr.puts "Error: can only specify one of --no-jit and --ftl-jit"
- exit 1
-end
-
if $doNotMessWithVMPath
$jscPath = Pathname.new(jscArg)
else
@@ -813,27 +803,27 @@
end
def runDefaultFTL
- run("default-ftl", *FTL_OPTIONS) if $enableFTL
+ run("default-ftl", *FTL_OPTIONS)
end
def runFTLNoCJIT
- run("ftl-no-cjit", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runFTLNoCJITValidate
- run("ftl-no-cjit-validate-sampling-profiler", "--validateGraph=true", "--useSamplingProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit-validate-sampling-profiler", "--validateGraph=true", "--useSamplingProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runFTLNoCJITNoPutStackValidate
- run("ftl-no-cjit-no-put-stack-validate", "--validateGraph=true", "--usePutStackSinking=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit-no-put-stack-validate", "--validateGraph=true", "--usePutStackSinking=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runFTLNoCJITNoInlineValidate
- run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runFTLNoCJITOSRValidation
- run("ftl-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runDFGEager
@@ -845,15 +835,15 @@
end
def runFTLEager
- run("ftl-eager", *(FTL_OPTIONS + EAGER_OPTIONS)) if $enableFTL
+ run("ftl-eager", *(FTL_OPTIONS + EAGER_OPTIONS))
end
def runFTLEagerNoCJITValidate
- run("ftl-eager-no-cjit", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
+ run("ftl-eager-no-cjit", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
end
def runFTLEagerNoCJITOSRValidation
- run("ftl-eager-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
+ run("ftl-eager-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
end
def runAlwaysTriggerCopyPhase
@@ -869,11 +859,11 @@
end
def runFTLNoCJITNoAccessInlining
- run("ftl-no-cjit-no-access-inlining", "--useAccessInlining=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit-no-access-inlining", "--useAccessInlining=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runFTLNoCJITSmallPool
- run("ftl-no-cjit-small-pool", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ run("ftl-no-cjit-small-pool", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runMiscNoCJITTest(*options)
@@ -938,7 +928,7 @@
end
def defaultQuickRun
- if $enableFTL and $jitTests
+ if $jitTests
runDefaultFTL
runFTLNoCJITValidate
else
@@ -1024,13 +1014,8 @@
return
end
- if $enableFTL
- run("ftl-no-cjit-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
- run("ftl-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS))
- else
- run("no-cjit-type-profiler", "--useTypeProfiler=true", *NO_CJIT_OPTIONS)
- run("type-profiler", "--useTypeProfiler=true")
- end
+ run("ftl-no-cjit-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
+ run("ftl-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS))
end
def runControlFlowProfiler
@@ -1038,11 +1023,7 @@
return
end
- if $enableFTL
- run("ftl-no-cjit-type-profiler", "--useControlFlowProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
- else
- run("no-cjit-type-profiler", "--useControlFlowProfiler=true", *NO_CJIT_OPTIONS)
- end
+ run("ftl-no-cjit-type-profiler", "--useControlFlowProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runTest262(mode, exception, includeFiles, flags)
@@ -1128,14 +1109,12 @@
run("no-cjit-validate-phases-modules", "-m", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *NO_CJIT_OPTIONS)
run("dfg-eager-modules", "-m", *EAGER_OPTIONS)
run("dfg-eager-no-cjit-validate-modules", "-m", "--validateGraph=true", *(NO_CJIT_OPTIONS + EAGER_OPTIONS))
- if $enableFTL
- run("default-ftl-modules", "-m", *FTL_OPTIONS)
- run("ftl-no-cjit-validate-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
- run("ftl-no-cjit-no-inline-validate-modules", "-m", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
- run("ftl-eager-modules", "-m", *(FTL_OPTIONS + EAGER_OPTIONS))
- run("ftl-eager-no-cjit-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
- run("ftl-no-cjit-small-pool-modules", "-m", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
- end
+ run("default-ftl-modules", "-m", *FTL_OPTIONS)
+ run("ftl-no-cjit-validate-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
+ run("ftl-no-cjit-no-inline-validate-modules", "-m", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
+ run("ftl-eager-modules", "-m", *(FTL_OPTIONS + EAGER_OPTIONS))
+ run("ftl-eager-no-cjit-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
+ run("ftl-no-cjit-small-pool-modules", "-m", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runLayoutTest(kind, *options)
@@ -1174,15 +1153,15 @@
end
def runLayoutTestDefaultFTL
- runLayoutTest("ftl", "--testTheFTL=true", *FTL_OPTIONS) if $enableFTL
+ runLayoutTest("ftl", "--testTheFTL=true", *FTL_OPTIONS)
end
def runLayoutTestFTLNoCJIT
- runLayoutTest("ftl-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+ runLayoutTest("ftl-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runLayoutTestFTLEagerNoCJIT
- runLayoutTest("ftl-eager-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
+ runLayoutTest("ftl-eager-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
end
def noFTLRunLayoutTest
@@ -1293,7 +1272,7 @@
end
def runMozillaTestDefaultFTL(mode, *extraFiles)
- runMozillaTest("ftl", mode, extraFiles, *FTL_OPTIONS) if $enableFTL
+ runMozillaTest("ftl", mode, extraFiles, *FTL_OPTIONS)
end
def runMozillaTestLLInt(mode, *extraFiles)
@@ -1309,11 +1288,11 @@
end
def runMozillaTestFTLEagerNoCJITValidatePhases(mode, *extraFiles)
- runMozillaTest("ftl-eager-no-cjit-validate-phases", mode, extraFiles, "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
+ runMozillaTest("ftl-eager-no-cjit-validate-phases", mode, extraFiles, "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
end
def defaultQuickRunMozillaTest(mode, *extraFiles)
- if $enableFTL and $jitTests
+ if $jitTests
runMozillaTestDefaultFTL(mode, *extraFiles)
runMozillaTestFTLEagerNoCJITValidatePhases(mode, *extraFiles)
else
@@ -1348,15 +1327,15 @@
end
def runNoisyTestDefaultFTL
- runNoisyTest("ftl", *FTL_OPTIONS) if $enableFTL
+ runNoisyTest("ftl", *FTL_OPTIONS)
end
def runNoisyTestNoCJIT
- runNoisyTest($enableFTL ? "ftl-no-cjit" : "no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(($enableFTL ? FTL_OPTIONS : []) + NO_CJIT_OPTIONS))
+ runNoisyTest("ftl-no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
end
def runNoisyTestEagerNoCJIT
- runNoisyTest($enableFTL ? "ftl-eager-no-cjit" : "eager-no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(($enableFTL ? FTL_OPTIONS : []) + NO_CJIT_OPTIONS + EAGER_OPTIONS))
+ runNoisyTest("ftl-eager-no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
end
def defaultRunNoisyTest
@@ -2040,13 +2019,6 @@
end
end
-if $enableFTL and ENV["JSCTEST_timeout"] or !ifJSCArgIsntProvidedAreWeReleaseBuild
- # Currently, using the FTL is a performance regression particularly in real
- # (i.e. non-loopy) benchmarks. Account for this in the timeout.
- # Increase the timeout for debug builds too. (--debug command line option)
- ENV["JSCTEST_timeout"] = (ENV["JSCTEST_timeout"].to_i * 2).to_s
-end
-
if ENV["JSCTEST_timeout"]
# In the worst case, the processors just interfere with each other.
# Increase the timeout proportionally to the number of processors.
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
