Closed Bug 1157354 Opened 9 years ago Closed 9 years ago

Add --tbpl-debug to jstests.py to run some faster variants of --tbpl on spidermonkey shell debug builds.

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox-esr38 --- fixed

People

(Reporter: arai, Assigned: arai)

Details

Attachments

(2 files)

Part 1: Add --tbpl-debug option to jstests.py.
3.98 KB, patch
sfink
: review+
Details | Diff | Splinter Review
Part 2: Pass --tbpl and --tbpl-debug to more spidermonkey shell build variants.
1.28 KB, patch
sfink
: review+
Details | Diff | Splinter Review 
Separated from bug 1156045.

Currently --tbpl is passed only to jstests.py for warnaserr build, in autospider.sh, to avoid timeout (bug 1101662 comment #11).
> if [[ "$VARIANT" = "warnaserr" ]]; then
>     export JSTESTS_EXTRA_ARGS=--tbpl
> fi

As next step, we could run some faster variants (maybe --*-eager flags?) on debug builds too.
here are results of time taken by each variant on local linux64 warnaserrdebug build,
so, 2 variants with --ion-eager is slower than others.
What's the priority between all of them? do we need to run at least one --ion-eager?

args: ""
  real  4m49.292s
  user  16m15.692s
  sys   1m25.016s

args: "--ion-eager --ion-offthread-compile=off"
  real  12m53.571s
  user  48m25.272s
  sys   1m44.112s

args: "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads"
  real  13m54.974s
  user  52m45.412s
  sys   1m51.644s

args: "--baseline-eager"
  real  5m33.258s
  user  19m35.468s
  sys   1m30.648s

args: "--baseline-eager --no-fpu"
  real  5m33.946s
  user  19m38.192s
  sys   1m29.620s

args: "--no-baseline --no-ion"
  real  8m29.717s
  user  31m24.040s
  sys   1m23.788s
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2430d5b03fb2

Added --tbpl-debug option (this should be renamed though), which adds following flags, and currently no timeout happens.
If other flags are more important, please tell me.
>+    [], # no flags, normal baseline and ion
>+    ['--ion-eager', '--ion-offthread-compile=off'], # implies --baseline-eager
>+    ['--baseline-eager'],
>+    ['--no-baseline', '--no-ion'],


build               | variant | option(after) | before | after
--------------------+---------+---------------+--------+----------
Linux opt           | e       | --tbpl        | -      | 39 [*1]
Linux debug         | arm     | --tbpl-debug  | 67     | 131
Linux debug         | e       | --tbpl-debug  | 35     | 70
Linux x64 opt       | e       | --tbpl        | 37     | 40 [*1]
Linux x64 debug     | cgc     |               | -      | 124 [*1]
Linux x64 debug     | e       | --tbpl-debug  | 39     | 69
Linux x64 debug     | r       | --tbpl-debug  | 65     | 130
Windows XP opt      | cgc     |               | -      | 122 [*1]
Windows XP opt      | p       | --tbpl        | 43     | 57
Windows XP debug    | cgc     |               | -      | 122 [*1]
Windows XP debug    | p       | --tbpl-debug  | 60     | 113
Windows 8 x64 debug | cgc     |               | -      | 125 [*1]
Windows 8 x64 debug | p       | --tbpl-debug  | ?      | 98

*1: no change in this patch


Linux debug SM(e) and Linux x64 debug SM(e) could be run with --tbpl.
Others are already too long, I think.
oops, please ignore the number in linux64 opt SM(e) before.
Okay, both SM(e) debug builds passed with --tbpl.
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cf5c7772c20&exclusion_profile=false

build               | variant | option(after) | before | after
--------------------+---------+---------------+--------+------
Linux opt           | e       | --tbpl        | -      | 41 [*1]
Linux debug         | arm     | --tbpl-debug  | 67     | 137
Linux debug         | e       | --tbpl        | 35     | 92
Linux x64 opt       | e       | --tbpl        | -      | 40 [*1]
Linux x64 debug     | cgc     |               | -      | 142 [*1]
Linux x64 debug     | e       | --tbpl        | 39     | 94
Linux x64 debug     | r       | --tbpl-debug  | 65     | 131
Windows XP opt      | cgc     |               | -      | 75 [*1]
Windows XP opt      | p       | --tbpl        | 43     | 62
Windows XP debug    | cgc     |               | -      | 168 [*1]
Windows XP debug    | p       | --tbpl-debug  | 60     | 116
Windows 8 x64 debug | cgc     |               | -      | 134 [*1]
Windows 8 x64 debug | p       | --tbpl-debug  | ?      | 102

*1: no change in this patch

Is it better to fix the name of --tbpl and --tinderbox option before this, and add proper name for the option for this patch?
Assignee: nobody → arai.unmht
Attachment #8599845 - Flags: review?(sphink)
Comment on attachment 8599845 [details] [diff] [review]
Part 1: Add --tbpl-debug option to jstests.py.

Review of attachment 8599845 [details] [diff] [review]:
-----------------------------------------------------------------

I'd vote for landing this as-is, then following up with a patch to rename. I'm tempted to reuse the obsolete --jitflags. Perhaps --tbpl => --jitflags=all, --tbpl-debug => --jitflags=debug? (Or use --loop-jitflags instead of --jitflags?)

::: js/src/tests/lib/tests.py
@@ +24,5 @@
> +TBPL_DEBUG_FLAGS = [
> +    [], # no flags, normal baseline and ion
> +    ['--ion-eager', '--ion-offthread-compile=off'], # implies --baseline-eager
> +    ['--baseline-eager'],
> +    ['--no-baseline', '--no-ion'],

Hm. Do we need the --no-baseline --no-ion one? I assume it's by far the slowest. I'm inclined to leave it out.

Note that SM(cgc) is passing --exclude-file=cgc-jstests-slow.txt, which is why it is no longer *insanely* slow.
Attachment #8599845 - Flags: review?(sphink) → review+
Comment on attachment 8599846 [details] [diff] [review]
Part 2: Pass --tbpl and --tbpl-debug to more spidermonkey shell build variants.

Review of attachment 8599846 [details] [diff] [review]:
-----------------------------------------------------------------

We should probably come up with a good way of figuring out what the true optimum setup is here (providing a mixture of fast builds that return results quickly, debug builds that give useful assertions, etc.) But I agree with you that this is better than what we have now.
Attachment #8599846 - Flags: review?(sphink) → review+
Thank you for reviewing.
Removed "--no-baseline --no-ion".

for renaming, filed as bug 1161410.
https://hg.mozilla.org/mozilla-central/rev/561c7f79f2a5
https://hg.mozilla.org/mozilla-central/rev/54333627915b
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: