Changeset 271179 in webkit

Timestamp:

Jan 5, 2021, 3:14:21 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC] allow stress tests to opt out of parallel execution
​https://bugs.webkit.org/show_bug.cgi?id=213373

Patch by Angelos Oikonomopoulos <Angelos Oikonomopoulos> on 2021-01-05
Reviewed by Yusuke Suzuki.

On memory-limited devices, some JSC stress tests may intermittently OOM when
they get scheduled along with another heavy JSC stress test. However, the tests
might be able to complete if run on their own.

This patch adds a serial! directive that causes a JSC stress test to only ever
be scheduled to run by itself. It's currently unused and needs to be enabled on
a test-by-test basis.

• Scripts/run-jsc-stress-tests:
• Scripts/webkitruby/jsc-stress-test-writer-default.rb:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/run-jsc-stress-tests (modified) (7 diffs)
• Scripts/webkitruby/jsc-stress-test-writer-default.rb (modified) (3 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2021-01-05  Angelos Oikonomopoulos  <angelos@igalia.com>
+
+        [JSC] allow stress tests to opt out of parallel execution
+        https://bugs.webkit.org/show_bug.cgi?id=213373
+
+        Reviewed by Yusuke Suzuki.
+
+        On memory-limited devices, some JSC stress tests may intermittently OOM when
+        they get scheduled along with another heavy JSC stress test. However, the tests
+        might be able to complete if run on their own.
+
+        This patch adds a serial! directive that causes a JSC stress test to only ever
+        be scheduled to run by itself. It's currently unused and needs to be enabled on
+        a test-by-test basis.
+
+        * Scripts/run-jsc-stress-tests:
+        * Scripts/webkitruby/jsc-stress-test-writer-default.rb:
+
 2021-01-05  Ryan Haddad  <ryanhaddad@apple.com>
 

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

@@ -552 +552 @@
 COLLECT_CONTINUOUSLY_OPTIONS = shouldCollectContinuously? ? ["--collectContinuously=true", "--useGenerationalGC=false"] : []
 
+$serialRunlist = []
 $runlist = []
 
@@ -619 +620 @@
         errorHandler)
     plan.additionalEnv.push(*additionalEnv)
+    if $runCommandOptions[:serial]
+        # Add this to the list of tests to be run on their own, so
+        # that we can treat them specially when scheduling, but keep
+        # it in the $runlist for code that dosn't care about
+        # scheduling.
+        $serialRunlist << plan
+    end
+
     if $numChildProcesses > 1 and $runCommandOptions[:isSlow]
         $runlist.unshift plan
@@ -668 +677 @@
     $testSpecificRequiredOptions += ["-s"]
     $runCommandOptions[:crashOK] = true
+end
+
+def serial!
+    $runCommandOptions[:serial] = true
 end
 
@@ -2390 +2403 @@
 end
 
-def prepareGnuParallelTestRunner
-    path = $runnerDir + "parallel-tests"
+def prepareGnuParallelRunnerJobs(name, runlist, exclude)
+    path = $runnerDir + name
     FileUtils.mkdir_p($runnerDir)
 
     File.open(path, "w") {
         | outp |
-        $runlist.each {
+        runlist.each {
             | plan |
+            if exclude.has_key?(plan)
+                next
+            end
             outp.puts("./test_script_#{plan.index}")
         }
     }
+end
+
+def prepareGnuParallelTestRunner
+    serialTests = {}
+    $serialRunlist.each { |p| serialTests[p] = nil }
+    prepareGnuParallelRunnerJobs("parallel-tests", $runlist, serialTests)
+    prepareGnuParallelRunnerJobs("serial-tests", $serialRunlist, {})
 end
 
@@ -2462 +2485 @@
 end
 
-def runGnuParallelRunner
-    inputs = $runnerDir + "parallel-tests"
+def runGnuParallelRunner(inputs, options={})
     timeout = 300
     if ENV["JSCTEST_timeout"]
         timeout = ENV["JSCTEST_timeout"].to_f.ceil.to_i
+    end
+    # We add 1 to make sure we always have waiting jobs and
+    # don't run into stalls due to ssh latency. However, we
+    # want to respect numChildProcesses, so we don't just use
+    # the -j +1 GNU parallel idiom.
+    parallelJobsOnEachHost = $numChildProcesses + 1
+    if options[:parallelJobsOnEachHost]
+      parallelJobsOnEachHost = options[:parallelJobsOnEachHost]
     end
     withGnuParallelSshLoginFile {
@@ -2472 +2502 @@
         cmd = [
             "parallel",
-            # We add 1 to make sure we always have waiting jobs and
-            # don't run into stalls due to ssh latency. However, we
-            # want to respect numChildProcesses, so we don't just use
-            # the -j +1 GNU parallel idiom.
-            "-j", "#{$numChildProcesses + 1}",
+            "-j", "#{parallelJobsOnEachHost}",
             "--retries 5",
             "--line-buffer", # we know our output is line-oriented
@@ -2509 +2535 @@
         raise "All remote hosts failed, giving up"
     end
-    runGnuParallelRunner
+    runGnuParallelRunner($runnerDir + "serial-tests",
+                         { :parallelJobsOnEachHost => 1})
+    runGnuParallelRunner($runnerDir + "parallel-tests")
     detectFailures
 end

trunk/Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb

@@ -306 +306 @@
 end
 
+def output_target(outp, plan, prereqs)
+    index = plan.index
+    target = "test_done_#{index}"
+    outp.puts "#{target}: #{prereqs.join(" ")}"
+    outp.puts "\tsh test_script_#{index}"
+    target
+end
+
 def prepareMakeTestRunner(remoteIndex)
     # The goals of our parallel test runner are scalability and simplicity. The
@@ -311 +319 @@
     # a full-time contributor just philosophising about parallel testing.
     #
-    # As such, we just pass off all of the hard work to 'make'. This creates a
-    # dummy directory ("$outputDir/.runner") in which we create a dummy
-    # Makefile. The Makefile has an 'all' rule that depends on all of the tests.
-    # That is, for each test we know we will run, there is a rule in the
-    # Makefile and 'all' depends on it. Running 'make -j <whatever>' on this
-    # Makefile results in 'make' doing all of the hard work:
+    # As such, we just pass off all of the hard work to 'make'. This
+    # creates a dummy directory ("$outputDir/.runner") in which we
+    # create a dummy Makefile. The Makefile has a 'parallel' rule that
+    # depends all tests, other than the ones marked 'serial'. The
+    # serial tests are arranged in a chain; the last target in the
+    # serial chain depends on 'parallel' and 'all' depends on the head
+    # of the chain. Running 'make -j <whatever>' on this Makefile
+    # results in 'make' doing all of the hard work:
     #
     # - Load balancing just works. Most systems have a great load balancer in
@@ -341 +351 @@
     # Even if two tests fail at the same time, since they're touching different
     # files we won't miss any failures.
-    runIndices = []
+    serialPlans = {}
+    $serialRunlist.each { |p| serialPlans[p] = nil }
+    runPlans = []
+    serialRunPlans = []
     $runlist.each {
         | plan |
         if !$remote or plan.index % $remoteHosts.length == remoteIndex
-            runIndices << plan.index
+            if serialPlans.has_key?(plan)
+                serialRunPlans << plan
+            else
+                runPlans << plan
+            end
         end
     }
-    
+
     File.open($runnerDir + "Makefile.#{remoteIndex}", "w") {
         | outp |
-        outp.puts("all: " + runIndices.map{|v| "test_done_#{v}"}.join(' '))
-        runIndices.each {
-            | index |
-            plan = $runlist[index]
-            outp.puts "test_done_#{index}:"
-            outp.puts "\tsh test_script_#{plan.index}"
+        if serialRunPlans.empty?
+            outp.puts("all: parallel")
+        else
+            serialPrereq = "test_done_#{serialRunPlans[-1].index}"
+            outp.puts("all: #{serialPrereq}")
+            prev_target = "parallel"
+            serialRunPlans.each {
+                | plan |
+                prev_target = output_target(outp, plan, [prev_target])
+            }
+        end
+        parallelTargets = runPlans.collect {
+            | plan |
+            output_target(outp, plan, [])
         }
+        outp.puts("parallel: " + parallelTargets.join(" "))
     }
 end