⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 203440 in webkit


Ignore:
Timestamp:
Jul 19, 2016, 6:45:25 PM (10 years ago)
Author:
fpizlo@apple.com
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/JavaScriptCore:

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-javascriptcore-tests:

(runJSCStressTests):

  • Scripts/run-jsc-stress-tests:
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r203421 r203440  
     12016-07-19  Filip Pizlo  <fpizlo@apple.com>
     2
     3        The default testing mode should not involve disabling the FTL JIT
     4        https://bugs.webkit.org/show_bug.cgi?id=159929
     5
     6        Rubber stamped by Mark Lam and Saam Barati.
     7       
     8        Use the new powers to make some tests run only in the default configuration (i.e. FTL,
     9        concurrent JIT).
     10
     11        * tests/mozilla/mozilla-tests.yaml:
     12
    1132016-07-19  Keith Miller  <keith_miller@apple.com>
    214

  • trunk/Source/JavaScriptCore/tests/mozilla/mozilla-tests.yaml

    r197962 r203440  
    593593  cmd: defaultRunMozillaTest :normal, "../shell.js"
    594594- path: ecma/FunctionObjects/15.3.1.1-3.js
    595   cmd: defaultRunMozillaTest :normal, "../shell.js"
     595  cmd: runMozillaTestDefaultFTL :normal, "../shell.js"
    596596- path: ecma/FunctionObjects/15.3.2.1-1.js
    597597  cmd: defaultRunMozillaTest :normal, "../shell.js"
    … …  
    599599  cmd: defaultRunMozillaTest :normal, "../shell.js"
    600600- path: ecma/FunctionObjects/15.3.2.1-3.js
    601   cmd: defaultRunMozillaTest :normal, "../shell.js"
     601  cmd: runMozillaTestDefaultFTL :normal, "../shell.js"
    602602- path: ecma/FunctionObjects/15.3.3.1-1.js
    603603  cmd: defaultRunMozillaTest :normal, "../shell.js"
    … …  
    617617  cmd: defaultRunMozillaTest :normal, "../shell.js"
    618618- path: ecma/FunctionObjects/15.3.5-1.js
    619   cmd: defaultRunMozillaTest :normal, "../shell.js"
     619  cmd: runMozillaTestDefaultFTL :normal, "../shell.js"
    620620- path: ecma/FunctionObjects/15.3.5-2.js
    621621  cmd: defaultRunMozillaTest :normal, "../shell.js"

  • trunk/Tools/ChangeLog

    r203436 r203440  
     12016-07-19  Filip Pizlo  <fpizlo@apple.com>
     2
     3        The default testing mode should not involve disabling the FTL JIT
     4        https://bugs.webkit.org/show_bug.cgi?id=159929
     5
     6        Rubber stamped by Mark Lam and Saam Barati.
     7       
     8        It used to be the case that most actively maintained ports did not have the FTL JIT enabled.
     9        Heck, for most of the FTL's initial development, it wasn't enabled anywhere. So, testing the
     10        FTL was not the default. You had to enable it with an option.
     11       
     12        For some reason we have kept this arrangement even though the FTL JIT is now the default on
     13        all of the major ports. This has become a serious pain. For example, it's useful to be able
     14        to say that a test should only run in the default config that is representative of what a
     15        normal user would see if they ran JSC. Clearly, this would be a config that does not
     16        explicitly disable the FTL JIT on the command line. However, if you try to specify this then
     17        your test won't run at all if the --ftl-jit option is not passed. That's dangerous!
     18       
     19        So, this change gets rid of all of this logic. I think it's better to get rid of it then to
     20        try to fix it, because:
     21       
     22        - I don't know what the fix would look like. Presumably it would ensure that ports that don't
     23          have the FTL enabled never run any tests that explicitly disable the FTL, since that
     24          doesn't do anything. The code is not really structured to allow this.
     25       
     26        - It benefits a minority of clients. Three build bots run tests in a config that has the FTL
     27          disabled in testing. On the other hand there are already build bots that do FTL tests
     28          despite having the FTL disabled by virtue of being a 32-bit platform. So, maybe rather than
     29          preserving this broken feature, we should create something that (a) acknowledges the fact
     30          that the FTL is the default on those platforms that support it and (b) avoids running
     31          no-ftl tests on precisely those platforms that don't have FTL.
     32       
     33        - To the extent that some bots benefited from disabling FTL tests, they were doing it by
     34          relying on a feature that was never meant to stick around. The FTL JIT is meant to be the
     35          default configuration. Disabling the FTL JIT is the non-default. So, we shouldn't be
     36          pretending that the FTL JIT is not the default just because some bots used that as an
     37          optimization.
     38       
     39        This change allows me to speed up some debug tests and paint some bots green.
     40
     41        * Scripts/run-javascriptcore-tests:
     42        (runJSCStressTests):
     43        * Scripts/run-jsc-stress-tests:
     44
    1452016-07-19  Alex Christensen  <achristensen@webkit.org>
    246

  • trunk/Tools/Scripts/run-javascriptcore-tests

    r202689 r203440  
    6666my $gmallocDefaultPath = "/usr/lib/libgmalloc.dylib";
    6767
    68 my $enableFTL = isAppleMacWebKit() || isX86_64() && (isGtk() || isEfl());
    6968my $createTarball = 0;
    7069my $remoteHost = 0;
    … …  
    149148    'extra-tests=s' => \$extraTests,
    150149    'build!' => \$buildJSC,
    151     'ftl-jit!' => \$enableFTL,
    152150    'testapi!' => \$runTestAPI,
    153151    'jsc-stress!' => \$runJSCStress,
    … …  
    172170my @buildArgs = @ARGV;
    173171
    174 # The --ftl-jit argument gets passed as a build argument.
    175 if ($enableFTL) {
    176     push(@buildArgs, '--ftl-jit');
    177 }
    178 
    179172if ($showHelp) {
    180173   print STDERR $usage;
    … …  
    211204my $productDir = jscProductDir();
    212205$ENV{DYLD_FRAMEWORK_PATH} = $productDir;
    213 $ENV{JSCTEST_timeout} = 60 unless $ENV{JSCTEST_timeout}; # Set a 60 second timeout on all jsc tests (if environment variable not defined already).
     206$ENV{JSCTEST_timeout} = 120 unless $ENV{JSCTEST_timeout}; # Set a 120 second timeout on all jsc tests (if environment variable not defined already).
    214207$ENV{TZ}="US/Pacific"; # Some tests fail if the time zone is not set to US/Pacific (<https://webkit.org/b/136363>)
    215208setPathForRunningWebKitApp(\%ENV) if isCygwin();
    … …  
    317310    if (!$runJITStressTests) {
    318311        push(@jscStressDriverCmd, "--no-jit");
    319     } else {
    320         if ($enableFTL) {
    321             push(@jscStressDriverCmd, "--ftl-jit");
    322         }
    323312    }
    324313    if ($createTarball) {

  • trunk/Tools/Scripts/run-jsc-stress-tests

    r203332 r203440  
    9898$jscPath = nil
    9999$doNotMessWithVMPath = false
    100 $enableFTL = false
    101100$jitTests = true
    102101$memoryLimited = false
    … …  
    125124    puts "--no-copy                   Do not copy the JavaScriptCore build product before testing."
    126125    puts "                            --jsc specifies an already present JavaScriptCore to test."
    127     puts "--ftl-jit                   Indicate that we have the FTL JIT."
    128126    puts "--memory-limited            Indicate that we are targeting the test for a memory limited device."
    129127    puts "                            Skip tests tagged with //@large-heap"
    … …  
    159157               ['--jsc', '-j', GetoptLong::REQUIRED_ARGUMENT],
    160158               ['--no-copy', GetoptLong::NO_ARGUMENT],
    161                ['--ftl-jit', GetoptLong::NO_ARGUMENT],
    162159               ['--memory-limited', GetoptLong::NO_ARGUMENT],
    163160               ['--no-jit', GetoptLong::NO_ARGUMENT],
    … …  
    189186    when '--output-dir'
    190187        $outputDir = Pathname.new(arg)
    191     when '--ftl-jit'
    192         $enableFTL = true
    193188    when '--memory-limited'
    194189        $memoryLimited = true
    … …  
    277272end
    278273
    279 if $enableFTL and !$jitTests
    280     $stderr.puts "Error: can only specify one of --no-jit and --ftl-jit"
    281     exit 1
    282 end
    283 
    284274if $doNotMessWithVMPath
    285275    $jscPath = Pathname.new(jscArg)
    … …  
    814804
    815805def runDefaultFTL
    816     run("default-ftl", *FTL_OPTIONS) if $enableFTL
     806    run("default-ftl", *FTL_OPTIONS)
    817807end
    818808
    819809def runFTLNoCJIT
    820     run("ftl-no-cjit", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     810    run("ftl-no-cjit", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    821811end
    822812
    823813def runFTLNoCJITValidate
    824     run("ftl-no-cjit-validate-sampling-profiler", "--validateGraph=true", "--useSamplingProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     814    run("ftl-no-cjit-validate-sampling-profiler", "--validateGraph=true", "--useSamplingProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    825815end
    826816
    827817def runFTLNoCJITNoPutStackValidate
    828     run("ftl-no-cjit-no-put-stack-validate", "--validateGraph=true", "--usePutStackSinking=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     818    run("ftl-no-cjit-no-put-stack-validate", "--validateGraph=true", "--usePutStackSinking=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    829819end
    830820
    831821def runFTLNoCJITNoInlineValidate
    832     run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     822    run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    833823end
    834824
    835825def runFTLNoCJITOSRValidation
    836     run("ftl-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     826    run("ftl-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    837827end
    838828
    … …  
    846836
    847837def runFTLEager
    848     run("ftl-eager", *(FTL_OPTIONS + EAGER_OPTIONS)) if $enableFTL
     838    run("ftl-eager", *(FTL_OPTIONS + EAGER_OPTIONS))
    849839end
    850840
    851841def runFTLEagerNoCJITValidate
    852     run("ftl-eager-no-cjit", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
     842    run("ftl-eager-no-cjit", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
    853843end
    854844
    855845def runFTLEagerNoCJITOSRValidation
    856     run("ftl-eager-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
     846    run("ftl-eager-no-cjit-osr-validation", "--validateFTLOSRExitLiveness=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
    857847end
    858848
    … …  
    870860
    871861def runFTLNoCJITNoAccessInlining
    872     run("ftl-no-cjit-no-access-inlining", "--useAccessInlining=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     862    run("ftl-no-cjit-no-access-inlining", "--useAccessInlining=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    873863end
    874864
    875865def runFTLNoCJITSmallPool
    876     run("ftl-no-cjit-small-pool", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     866    run("ftl-no-cjit-small-pool", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    877867end
    878868
    … …  
    939929
    940930def defaultQuickRun
    941     if $enableFTL and $jitTests
     931    if $jitTests
    942932        runDefaultFTL
    943933        runFTLNoCJITValidate
    … …  
    10251015    end
    10261016
    1027     if $enableFTL
    1028         run("ftl-no-cjit-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    1029         run("ftl-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS))
    1030     else
    1031         run("no-cjit-type-profiler", "--useTypeProfiler=true", *NO_CJIT_OPTIONS)
    1032         run("type-profiler", "--useTypeProfiler=true")
    1033     end
     1017    run("ftl-no-cjit-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
     1018    run("ftl-type-profiler", "--useTypeProfiler=true", *(FTL_OPTIONS))
    10341019end
    10351020
    … …  
    10391024    end
    10401025
    1041     if $enableFTL
    1042         run("ftl-no-cjit-type-profiler", "--useControlFlowProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    1043     else
    1044         run("no-cjit-type-profiler", "--useControlFlowProfiler=true", *NO_CJIT_OPTIONS)
    1045     end
     1026    run("ftl-no-cjit-type-profiler", "--useControlFlowProfiler=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    10461027end
    10471028
    … …  
    11291110    run("dfg-eager-modules", "-m", *EAGER_OPTIONS)
    11301111    run("dfg-eager-no-cjit-validate-modules", "-m", "--validateGraph=true", *(NO_CJIT_OPTIONS + EAGER_OPTIONS))
    1131     if $enableFTL
    1132         run("default-ftl-modules", "-m", *FTL_OPTIONS)
    1133         run("ftl-no-cjit-validate-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    1134         run("ftl-no-cjit-no-inline-validate-modules", "-m", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    1135         run("ftl-eager-modules", "-m", *(FTL_OPTIONS + EAGER_OPTIONS))
    1136         run("ftl-eager-no-cjit-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
    1137         run("ftl-no-cjit-small-pool-modules", "-m", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    1138     end
     1112    run("default-ftl-modules", "-m", *FTL_OPTIONS)
     1113    run("ftl-no-cjit-validate-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
     1114    run("ftl-no-cjit-no-inline-validate-modules", "-m", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
     1115    run("ftl-eager-modules", "-m", *(FTL_OPTIONS + EAGER_OPTIONS))
     1116    run("ftl-eager-no-cjit-modules", "-m", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
     1117    run("ftl-no-cjit-small-pool-modules", "-m", "--jitMemoryReservationSize=50000", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    11391118end
    11401119
    … …  
    11751154
    11761155def runLayoutTestDefaultFTL
    1177     runLayoutTest("ftl", "--testTheFTL=true", *FTL_OPTIONS) if $enableFTL
     1156    runLayoutTest("ftl", "--testTheFTL=true", *FTL_OPTIONS)
    11781157end
    11791158
    11801159def runLayoutTestFTLNoCJIT
    1181     runLayoutTest("ftl-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
     1160    runLayoutTest("ftl-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    11821161end
    11831162
    11841163def runLayoutTestFTLEagerNoCJIT
    1185     runLayoutTest("ftl-eager-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
     1164    runLayoutTest("ftl-eager-no-cjit", "--testTheFTL=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
    11861165end
    11871166
    … …  
    12941273
    12951274def runMozillaTestDefaultFTL(mode, *extraFiles)
    1296     runMozillaTest("ftl", mode, extraFiles, *FTL_OPTIONS) if $enableFTL
     1275    runMozillaTest("ftl", mode, extraFiles, *FTL_OPTIONS)
    12971276end
    12981277
    … …  
    13101289
    13111290def runMozillaTestFTLEagerNoCJITValidatePhases(mode, *extraFiles)
    1312     runMozillaTest("ftl-eager-no-cjit-validate-phases", mode, extraFiles, "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS)) if $enableFTL
     1291    runMozillaTest("ftl-eager-no-cjit-validate-phases", mode, extraFiles, "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
    13131292end
    13141293
    13151294def defaultQuickRunMozillaTest(mode, *extraFiles)
    1316     if $enableFTL and $jitTests
     1295    if $jitTests
    13171296        runMozillaTestDefaultFTL(mode, *extraFiles)
    13181297        runMozillaTestFTLEagerNoCJITValidatePhases(mode, *extraFiles)
    … …  
    13491328
    13501329def runNoisyTestDefaultFTL
    1351     runNoisyTest("ftl", *FTL_OPTIONS) if $enableFTL
     1330    runNoisyTest("ftl", *FTL_OPTIONS)
    13521331end
    13531332
    13541333def runNoisyTestNoCJIT
    1355     runNoisyTest($enableFTL ? "ftl-no-cjit" : "no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(($enableFTL ? FTL_OPTIONS : []) + NO_CJIT_OPTIONS))
     1334    runNoisyTest("ftl-no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
    13561335end
    13571336
    13581337def runNoisyTestEagerNoCJIT
    1359     runNoisyTest($enableFTL ? "ftl-eager-no-cjit" : "eager-no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(($enableFTL ? FTL_OPTIONS : []) + NO_CJIT_OPTIONS + EAGER_OPTIONS))
     1338    runNoisyTest("ftl-eager-no-cjit", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS))
    13601339end
    13611340
    … …  
    20412020end
    20422021
    2043 if $enableFTL and ENV["JSCTEST_timeout"] or !ifJSCArgIsntProvidedAreWeReleaseBuild
    2044     # Currently, using the FTL is a performance regression particularly in real
    2045     # (i.e. non-loopy) benchmarks. Account for this in the timeout.
    2046     # Increase the timeout for debug builds too. (--debug command line option)
    2047     ENV["JSCTEST_timeout"] = (ENV["JSCTEST_timeout"].to_i * 2).to_s
    2048 end
    2049 
    20502022if ENV["JSCTEST_timeout"]
    20512023    # In the worst case, the processors just interfere with each other.
Note: See TracChangeset for help on using the changeset viewer.