Bug 159929 – The default testing mode should not involve disabling the FTL JIT

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ryan Haddad]

https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20JSC%20%28Tests%29/builds/3491/steps/jscore-test/logs/stdio

** The following JSC stress test failures have been introduced:
	mozilla-tests.yaml/ecma/FunctionObjects/15.3.1.1-3.js.mozilla-ftl-eager-no-cjit-validate-phases
	mozilla-tests.yaml/ecma/FunctionObjects/15.3.2.1-3.js.mozilla-ftl-eager-no-cjit-validate-phases
	mozilla-tests.yaml/ecma/FunctionObjects/15.3.5-1.js.mozilla-ftl-eager-no-cjit-validate-phases

[Filip Pizlo]

Looking at this now.

It's OK to roll it out.  But it's likely I'll have a fix within an hour.

[Filip Pizlo]

Hmm, these are just timeouts.  I'm not sure yet what the best thing to do is.  Debug timeouts are not very interesting.

[Filip Pizlo]

Looking at these now.  I'm tempted to say that we should just not run these tests in the ftl-eager-no-cjit-validate-phases configuration.  I think that the increase in DFG/FTL coverage, particularly with the addition of eval support, has made these tests much slower.  That's expected behavior for eager compilation tests: they run slower when the compiler gets better.

[Filip Pizlo]

Wow, this test:

ecma/FunctionObjects/15.3.1.1-3.js

takes a really long time when running in debug with force eager compilation.  That's probably not a bug in the VM, but I'll check anyway.

Most likely I'll end up disabling forceEagerCompilation runs of this test.

[Filip Pizlo]

(In reply to comment #4)
> Wow, this test:
> 
> ecma/FunctionObjects/15.3.1.1-3.js
> 
> takes a really long time when running in debug with force eager compilation.
> That's probably not a bug in the VM, but I'll check anyway.
> 
> Most likely I'll end up disabling forceEagerCompilation runs of this test.

Wow, 1 minute 33 seconds on my machine!

[Filip Pizlo]

(In reply to comment #5)
> (In reply to comment #4)
> > Wow, this test:
> > 
> > ecma/FunctionObjects/15.3.1.1-3.js
> > 
> > takes a really long time when running in debug with force eager compilation.
> > That's probably not a bug in the VM, but I'll check anyway.
> > 
> > Most likely I'll end up disabling forceEagerCompilation runs of this test.
> 
> Wow, 1 minute 33 seconds on my machine!

Heh it spends 30 seconds compiling some crazy DFG function.  That's not a real bug.  It's slow because we're in debug and we're compiling synchronously, 'cause that's the test mode.

[Filip Pizlo]

Even in release, this test takes 5.7sec.  That's way too long!

[Filip Pizlo]

It takes 50ms without forceEagerCompilation, so it's not a VM bug.

[Filip Pizlo]

Created attachment 284076 [details]
the patch

[Mark Lam]

Comment on attachment 284076 [details]
the patch

rs=me

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/203440

[Csaba Osztrogonác]

(In reply to comment #11)
> Landed in https://trac.webkit.org/changeset/203440

Great, now 32 bit bots (and AArch64 Linux) became terrible slow, because they run FTL tests just for fun.

[Csaba Osztrogonác]

(In reply to comment #12)
> (In reply to comment #11)
> > Landed in https://trac.webkit.org/changeset/203440
> 
> Great, now 32 bit bots (and AArch64 Linux) became terrible slow, because
> they run FTL tests just for fun.

Just a quick statistic:
- AArch64, ARMv7 Thumb2, ARMv7 Traditional and GTK ARM bots
now run 36.000 tests instead of 20.000, the extra 16.000 tests
don't add anything for testing, they are simply duplicated tests.
- runtime on AArch64 bot: 73 mins -> 112 mins
- runtime on ARMv7 Thumb2 bot: 109 mins -> 168 mins
- runtime on ARMv7 ARM traditional bot: 186 mins -> 260 mins
- runtime on ARM GTK (Thumb2) bot: 48 mins -> 76 mins

"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."

OK, so when do you plan to fix this completely broken jsc-stress-tests
script not to run extra tests on 32-bit platforms which won't have
FTL JIT ever in the future? It's not fair to increase the test runtime
on 32-bit platforms just to paint your extra slow debug bot green.

[Csaba Osztrogonác]

Not to mention that it completely broke the Apple Windows JSC testing
infrastructere.

[Michael Saboff]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Landed in https://trac.webkit.org/changeset/203440
> > 
> > Great, now 32 bit bots (and AArch64 Linux) became terrible slow, because
> > they run FTL tests just for fun.
> 
> Just a quick statistic:
> - AArch64, ARMv7 Thumb2, ARMv7 Traditional and GTK ARM bots
> now run 36.000 tests instead of 20.000, the extra 16.000 tests
> don't add anything for testing, they are simply duplicated tests.
> - runtime on AArch64 bot: 73 mins -> 112 mins
> - runtime on ARMv7 Thumb2 bot: 109 mins -> 168 mins
> - runtime on ARMv7 ARM traditional bot: 186 mins -> 260 mins
> - runtime on ARM GTK (Thumb2) bot: 48 mins -> 76 mins
> 
> "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."
> 
> OK, so when do you plan to fix this completely broken jsc-stress-tests
> script not to run extra tests on 32-bit platforms which won't have
> FTL JIT ever in the future? It's not fair to increase the test runtime
> on 32-bit platforms just to paint your extra slow debug bot green.

Created <https://bugs.webkit.org/show_bug.cgi?id=160058> to track the increased testing time.

[Csaba Osztrogonác]

(In reply to comment #15)

> Created <https://bugs.webkit.org/show_bug.cgi?id=160058> to track the
> increased testing time.

Fix is in bug160033, waiting for review.