GMail broken on Windows builds with JS PGO enabled

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: RyanVM, Assigned: m_kato)

Tracking

Trunk
mozilla15
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Lately, I've been having numerous issues with GMail. When I send a message, the inbox often goes blank and clicking on folders on the side doesn't do anything. I've also had it where messages don't open when I click them. The only way to get it to stop acting glitchy is to reload the page. I've confirmed that these problems occur on a clean profile.

The error console catches a few warnings after reloading (but none while it's being glitchy):
Timestamp: 1/25/2012 8:48:08 PM
Error: attempt to run compile-and-go script on a cleared scope
Source File: https://mail.google.com/mail/?ui=2&view=js&name=main,tlist&ver=GgkbWT1cpLY.en.&am=!K44dex2vU9fZQv2QwUQ3cOvs29WB0V7rOEb7Sqjjbx092iPTTWejs2AkSSLTvRoEbw
Line: 407

Timestamp: 1/25/2012 8:49:10 PM
Error: uncaught exception: CustomError: Error in protected function: too much recursion

I haven't nailed down a regression range yet.
Can you reproduce this in safe mode?
(Reporter)

Comment 2

6 years ago
Works fine in safe mode. Given that my clean profile had no extensions, I'm going to assume this is JIT-related. I'm installing mozregression now. We'll see what it finds.
(Reporter)

Comment 3

6 years ago
I can't reproduce with nightlies using mozregression. My personal builds use VC10 SP1 with JS PGO enabled. I'm going to do a tryserver push with JS PGO turned on to see if it reproduces with that build.

Comment 4

6 years ago
As for the "cleared global" error, I've seen that consistently in google apps.
Marking dep on ClearScope. That might not be the only problem though.
Depends on: 637099
(Reporter)

Comment 6

6 years ago
I did a Try run with JS PGO enabled and see the same issues. If someone wants to reproduce, try the build here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ryanvm@gmail.com-dfd45f64ac35/try-win32/

https://tbpl.mozilla.org/?tree=Try&rev=dfd45f64ac35
(Reporter)

Comment 7

6 years ago
I ran tests this time. Looks like PGO is in fact breaking something. Question is - compiler bug or "real" bug?
https://tbpl.mozilla.org/?tree=Try&rev=709b3fab47c0

In jsreftest:
BUGNUMBER: 380237
STATUS: Generator expressions - sudoku
optionsReset: caught [Exception... "Cannot modify properties of a WrappedNative"  nsresult: "0x80570034 (NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN)"  location: "JS frame :: file:///c:/talos-slave/test/build/jsreftest/tests/browser.js :: options :: line 211"  data: no]
REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_8/genexps/regress-380237-01.js | No test results reported. (SCRIPT)

In xpcshell:
TEST-INFO | c:\talos-slave\test\build\xpcshell\tests\toolkit\mozapps\extensions\test\xpcshell\test_general.js | running test ...

command timed out: 1200 seconds without output, attempting to kill
SIGKILL failed to kill process
using fake rc=-1
program finished with exit code -1
(Reporter)

Comment 8

6 years ago
Bisecting with hg revert (thank God for Try), I've narrowed down the regressing patch to bug 706914. With any luck, the same issue leading to comment 7's failures is the cause of the GMail issues (seeing as how they appear to go hand in hand).

Brian, I know that JS PGO is not really a supported build config, but there could very well be an actual issue here that PGO just happens to be poking in the right manner. It's worth noting that the same issue appears with both VC8 and VC10. I really hope that the time I've spent narrowing this down hasn't been a total waste.
Blocks: 706914
No longer depends on: 637099
(Reporter)

Updated

6 years ago
Blocks: 673518
(Reporter)

Comment 9

6 years ago
At Brian's request, I attempted a build changing CHUNK_LIMIT in methodjit/Compiler.cpp to 'uint32_t(-1)'. The test failures still reproduce and GMail still doesn't work properly.

https://tbpl.mozilla.org/?tree=Try&rev=b864c1ec4c1d
With the change above there shouldn't be any difference in generated jitcode vs. an earlier revision, and the problem is probably a PGO-related miscompilation somewhere in the C++ changes.  Does anyone know if there is a way to selectively disable PGO at a compilation unit (or finer) granularity?
(Reporter)

Comment 11

6 years ago
Looking at http://msdn.microsoft.com/en-us/library/ms235601.aspx, it appears that |#pragma optimize("", off)| might work on a function by function basis.
Yes, it looks like that was used in bug 563318 to disable optimization of a particular problematic function.
(Reporter)

Updated

6 years ago
Summary: Problems with GMail → GMail broken on Windows builds with JS PGO enabled
(Assignee)

Comment 13

5 years ago
Created attachment 627415 [details] [diff] [review]
pgo workaround

Comment 14

5 years ago
(In reply to Makoto Kato from comment #13)
> Created attachment 627415 [details] [diff] [review]
> pgo workaround

I remembered VC2010 use -fp:precise by default.

Some functions around use #pragma optimize("g", off) and #pragma optimize("g", off) maybe better than using -GL-.

Comment 15

5 years ago
(In reply to Makoto Kato from comment #13)
> Created attachment 627415 [details] [diff] [review]
> pgo workaround

I remembered VC2010 use -fp:precise by default.

Some functions around use #pragma optimize("g", off) and #pragma optimize("g", off) maybe better than using -GL-.(In reply to xunxun from comment #14)
> (In reply to Makoto Kato from comment #13)
> > Created attachment 627415 [details] [diff] [review]
> > pgo workaround
> 
> I remembered VC2010 use -fp:precise by default.
> 
> Some functions around use #pragma optimize("g", off) and #pragma
> optimize("g", off) maybe better than using -GL-.

use #pragma optimize("g", off) and #pragma optimize("g", off)

-->

use #pragma optimize("g", off) and #pragma optimize("g", on)
(Assignee)

Comment 16

5 years ago
Comment on attachment 627415 [details] [diff] [review]
pgo workaround

Passed on try server.
https://tbpl.mozilla.org/?tree=Try&rev=0e817439c08f

I think that some files should be except to PGO optimization due to x86 compiler issue. Also this workaround isn't needed on x64 compiler.
To be safety, use -GL- instead of #pragma.

And you know, since Win32 build uses mozjs.dll, I believe that no effect for xul.dll linking memory issue.
Attachment #627415 - Flags: review?(dmandelin)
Comment on attachment 627415 [details] [diff] [review]
pgo workaround

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

Seems reasonable. Thanks for looking into this--really tricky stuff.
Attachment #627415 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 18

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6a4384fca8e
Target Milestone: --- → mozilla15
(Assignee)

Updated

5 years ago
Assignee: general → m_kato
https://hg.mozilla.org/mozilla-central/rev/a6a4384fca8e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.