Last Comment Bug 721284 - GMail broken on Windows builds with JS PGO enabled
: GMail broken on Windows builds with JS PGO enabled
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal with 1 vote (vote)
: mozilla15
Assigned To: Makoto Kato [:m_kato]
:
:
Mentors:
Depends on:
Blocks: 673518 706914
  Show dependency treegraph
 
Reported: 2012-01-25 17:49 PST by Ryan VanderMeulen [:RyanVM]
Modified: 2012-05-31 05:51 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
pgo workaround (1.57 KB, patch)
2012-05-25 17:48 PDT, Makoto Kato [:m_kato]
dmandelin: review+
Details | Diff | Splinter Review

Description Ryan VanderMeulen [:RyanVM] 2012-01-25 17:49:48 PST
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.
Comment 1 Andrew McCreight [:mccr8] 2012-01-25 17:59:55 PST
Can you reproduce this in safe mode?
Comment 2 Ryan VanderMeulen [:RyanVM] 2012-01-25 18:06:43 PST
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.
Comment 3 Ryan VanderMeulen [:RyanVM] 2012-01-25 18:21:35 PST
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 Luke Wagner [:luke] 2012-01-25 18:25:30 PST
As for the "cleared global" error, I've seen that consistently in google apps.
Comment 5 David Mandelin [:dmandelin] 2012-01-25 18:38:36 PST
Marking dep on ClearScope. That might not be the only problem though.
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-01-26 14:30:03 PST
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
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-01-27 17:17:14 PST
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
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-01-29 05:01:28 PST
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.
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-01-31 14:19:11 PST
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
Comment 10 Brian Hackett (:bhackett) 2012-01-31 14:38:44 PST
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?
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-01-31 14:39:59 PST
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.
Comment 12 Andrew McCreight [:mccr8] 2012-02-02 14:08:45 PST
Yes, it looks like that was used in bug 563318 to disable optimization of a particular problematic function.
Comment 13 Makoto Kato [:m_kato] 2012-05-25 17:48:36 PDT
Created attachment 627415 [details] [diff] [review]
pgo workaround
Comment 14 xunxun 2012-05-25 18:01:23 PDT
(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 xunxun 2012-05-25 18:02:27 PDT
(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)
Comment 16 Makoto Kato [:m_kato] 2012-05-27 18:06:49 PDT
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.
Comment 17 David Mandelin [:dmandelin] 2012-05-29 11:37:56 PDT
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.
Comment 19 Ed Morley [:emorley] 2012-05-31 05:51:11 PDT
https://hg.mozilla.org/mozilla-central/rev/a6a4384fca8e

Note You need to log in before you can comment on or make changes to this bug.