Bug 149621 – REGRESSION(r190289): It made Speedometer/Full.html performance test fail

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

[Csaba Osztrogonác]

https://trac.webkit.org/changeset/190289 made Speedometer/Full.html performance test fail

Running Speedometer/Full.html (151 of 151)
ERROR: CONSOLE MESSAGE: line 208: Miss the info bar? Run TodoMVC from a server to avoid a cross-origin error.
FAILED

- EFL perf bot: https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/6952
- Apple perf bot: https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Perf%29/builds/3026

The latest known good revision is r190286. ( https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Perf%29/builds/3025 )
The first failing revision is r190291. ( https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/6952 )

I tried to bisect this small interval locally, but didn't manage, because this
test crashes almost always due to a serious GC related regression - bug149341.
( Unfortunately it seems nobody is interested in fixing crashes/regressions ... )

[Csaba Osztrogonác]

r190288 is the last good revision - https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Perf%29/builds/3031

[Michael Saboff]

I am able to reproduce this in both release and debug builds although the symptoms are a little different.

In a debug build, we are OSR exiting from a function due to BadType, but the real reason is that the tag registers (r14 & r15 on X86-64) do not have the tag constants.  it appears that they were not properly restored after a call (likely to an FTL function).  More debugging.

[Alexey Proskuryakov]

Should this be duped to bug 149647 now (the rollout bug)?

[Michael Saboff]

(In reply to comment #3)
> Should this be duped to bug 149647 now (the rollout bug)?

The rollout bug is closed.  I'll still work on the issue with this defect.  I'll add this defect and https://bugs.webkit.org/show_bug.cgi?id=149619 to bug 149647.

[Michael Saboff]

A little more detail.  We are calling a function that makes a varargs tail call.  When the FTL makes a tail call and it needs to take the slow path, it doesn't properly restore the callee save registers.  Moved the restoration of callee saves to common path before deciding to take the fast or slow path.

[Michael Saboff]

Created attachment 262192 [details]
Patch

[Michael Saboff]

Landed as part of change set r190370: <http://trac.webkit.org/changeset/190370>.

[Keith Miller]

Committed r190372: <http://trac.webkit.org/changeset/190372>

[Michael Saboff]

(In reply to comment #8)
> Committed r190372: <http://trac.webkit.org/changeset/190372>

This was really a change for an unrelated build fix.

[Csaba Osztrogonác]

(In reply to comment #7)
> Landed as part of change set r190370:
> <http://trac.webkit.org/changeset/190370>.

Reopen, because r190370 didn't fix the orignal issue, Speedometer/Full.html
still fails everywhere. Additionally it can't be an FTL issue, because
FTL isn't enabled on Linux platforms at all.

- https://build.webkit.org/builders/Apple%20Mavericks%20Release%20WK2%20%28Perf%29/builds/5900
- https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/6975

To make sure if r190370 is the culprit, I started force builds on perf bots:
- r190368 - https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Perf%29/builds/12
- r190370 - https://build.webkit.org/builders/Apple%20Mavericks%20Release%20WK2%20%28Perf%29/builds/5904

(r190369 is an unrelated change)

[Michael Saboff]

(In reply to comment #10)
> (In reply to comment #7)
> > Landed as part of change set r190370:
> > <http://trac.webkit.org/changeset/190370>.
> 
> Reopen, because r190370 didn't fix the orignal issue, Speedometer/Full.html
> still fails everywhere. Additionally it can't be an FTL issue, because
> FTL isn't enabled on Linux platforms at all.
> 
> -
> https://build.webkit.org/builders/
> Apple%20Mavericks%20Release%20WK2%20%28Perf%29/builds/5900
> -
> https://build.webkit.org/builders/EFL%20Linux%2064-
> bit%20Release%20WK2%20%28Perf%29/builds/6975
> 
> To make sure if r190370 is the culprit, I started force builds on perf bots:
> - r190368 -
> https://build.webkit.org/builders/
> Apple%20El%20Capitan%20Release%20WK2%20%28Perf%29/builds/12
> - r190370 -
> https://build.webkit.org/builders/
> Apple%20Mavericks%20Release%20WK2%20%28Perf%29/builds/5904
> 
> (r190369 is an unrelated change)

The patch that was landed solves the problem encountered on Mac builds.

Is it possible for you to run Speedometer/Full.html from a debug build in a debugger and post stack trace?

[Csaba Osztrogonác]

I didn't notice crashes (in release mode), but I get this console
message locally (EFL Linux x86_64) and you can see it on EFL and 
Apple Mac performance bots too:

ERROR: CONSOLE MESSAGE: line 208: Miss the info bar? Run TodoMVC from a server to avoid a cross-origin error.

After grepping a little bit, base.js:140 in SpeedoMeter 
is responsible for printing this message to console.

[Chris Dumez]

(In reply to comment #12)
> I didn't notice crashes (in release mode), but I get this console
> message locally (EFL Linux x86_64) and you can see it on EFL and 
> Apple Mac performance bots too:
> 
> ERROR: CONSOLE MESSAGE: line 208: Miss the info bar? Run TodoMVC from a
> server to avoid a cross-origin error.
> 
> After grepping a little bit, base.js:140 in SpeedoMeter 
> is responsible for printing this message to console.

Looks like:
https://bugs.webkit.org/show_bug.cgi?id=149720

[Csaba Osztrogonác]

*** Bug 149720 has been marked as a duplicate of this bug. ***

[Ryosuke Niwa]

*** Bug 149720 has been marked as a duplicate of this bug. ***

[Ryosuke Niwa]

<rdar://problem/22995488>

In order to reproduce this failure, revert http://trac.webkit.org/changeset/190535

[Ryosuke Niwa]

The bug is that console.info called in line 140 of base.js (there are a few copies of it inside PerformanceTests/Speedometer/) is reported erroneously at line 208.

Given:
138:    function getFile(file, callback) {
139:        if (!location.host) {
140:            return console.info('Miss the info bar? Run TodoMVC from a server to avoid a cross-origin error.');
141:        }

207:    redirect();
208:    getFile('learn.json', Learn);
209: })();

I'd imagine that the console.info in "getFile" is tail-call optimized into the caller, and erroneously reporting the line number.