Last Comment Bug 709914 - Slice out WebGL and the ANGLE compiler from libxul
: Slice out WebGL and the ANGLE compiler from libxul
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: 11 Branch
: x86 Linux
: -- normal (vote)
: mozilla11
Assigned To: Mike Hommey [:glandium]
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 709721
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-12 12:19 PST by JP Rosevear [:jpr]
Modified: 2012-03-11 10:48 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Slice out the ANGLE compiler from libxul on Windows (2.24 KB, patch)
2011-12-13 01:42 PST, Mike Hommey [:glandium]
khuey: review+
Details | Diff | Splinter Review
part 2: fix the variable access (2.11 KB, patch)
2011-12-14 15:22 PST, :Ehsan Akhgari
cpearce: review+
Details | Diff | Splinter Review
Slice out the ANGLE compiler from libxul on Windows (2.27 KB, patch)
2011-12-14 21:31 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review

Description JP Rosevear [:jpr] 2011-12-12 12:19:21 PST
In order to make libxul lighter and help move out of bug 709193, we could separate out ANGLE in a separate library.

According to glandium, we should probably put all this sliced out stuff into one library.
Comment 1 Benoit Jacob [:bjacob] (mostly away) 2011-12-12 12:41:54 PST
Notice that ANGLE consists of 3 parts:
 - the compiler
 - libGLESv2   (Windows-only)
 - libEGL      (Windows-only)

libGLESv2 and libEGL are already separate DLLs.

What we could do is split out the WebGL implementation and the ANGLE compiler (i.e. the only part of ANGLE that's currently in libxul) into a separate 'mozwebgl' library.
Comment 2 Mike Hommey [:glandium] 2011-12-12 12:58:04 PST
It would be better to put the whole of angle in one place (so, including libGLESv2 and libEGL, except if they are LoadLibrary()ed), and group that with the media libraries. I named the media library gkmedias.dll, but it could be renamed to anything.
Comment 3 Benoit Jacob [:bjacob] (mostly away) 2011-12-12 15:30:56 PST
I disagree with comment 2, some additional background: the libGLESv2.dll and libEGL.dll parts of ANGLE implement standards, they're interchangeable with other implementations of these standards. It's actually useful to be able to replace them with e.g. the ES drivers provided by ATI on Windows. So, keep them unchanged please.
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2011-12-12 15:46:27 PST
http://hg.mozilla.org/releases/mozilla-beta/rev/d2f1a400e430

keeping open for central and aurora
Comment 5 Jeff Gilbert [:jgilbert] 2011-12-12 16:34:04 PST
bjacob's post was a mt from bug 709947.
Comment 6 Mike Hommey [:glandium] 2011-12-13 01:42:32 PST
Created attachment 581206 [details] [diff] [review]
Slice out the ANGLE compiler from libxul on Windows

This folds the ANGLE compiler with the media libraries from bug 709721. I kept using the layout/media directory and the gkmedias library name. Pushed to try on all platforms, to confirm that nothing breaks as a result:
https://tbpl.mozilla.org/?noignore=1&tree=Try&rev=683e92eb1df0
Comment 7 Mike Hommey [:glandium] 2011-12-13 04:42:51 PST
Apparently, this breaks webgl tests :(
Comment 8 Jeff Gilbert [:jgilbert] 2011-12-14 05:06:07 PST
Some of those webgl fails look like problems with video. How does the try push look before the ANGLE split patch?
Comment 9 Mike Hommey [:glandium] 2011-12-14 08:36:37 PST
(In reply to Jeff Gilbert [:jgilbert] from comment #8)
> Some of those webgl fails look like problems with video. How does the try
> push look before the ANGLE split patch?

https://tbpl.mozilla.org/?tree=Try&rev=6493efb5b8e9
Comment 10 Benoit Jacob [:bjacob] (mostly away) 2011-12-14 08:41:31 PST
My first concern looking at this patch is that the ANGLE compiler files are added to 3 makefiles: the one you touched, and again in the 2 other makefiles

$ find . -name Makefile.in
./Makefile.in
./src/libGLESv2/Makefile.in
./src/libEGL/Makefile.in

So is the current patch compiling these in *both* libxul and the new lib?
Comment 11 Benoit Jacob [:bjacob] (mostly away) 2011-12-14 08:44:24 PST
Ignore comment 10, i'm stupid. these 2 other makefiles compile into other libraries, not into libxul, and it is fully intentional that they ship a copy of the angle compiler, as they need that code and we dont want (I guess) to have them depend on libxul.
Comment 12 Benoit Jacob [:bjacob] (mostly away) 2011-12-14 08:45:09 PST
Though... what we could do is compile the angle compiler into a standalone library and then have libxul, libEGL and libGLESv2 all link to it.
Comment 13 Mike Hommey [:glandium] 2011-12-14 08:46:57 PST
(In reply to Benoit Jacob [:bjacob] from comment #12)
> Though... what we could do is compile the angle compiler into a standalone
> library and then have libxul, libEGL and libGLESv2 all link to it.

That's basically what this is about, except libEGL and libGLESv2 statically link with angle with the patch here.
Comment 14 Benoit Jacob [:bjacob] (mostly away) 2011-12-14 14:25:02 PST
I've looked into these WebGL test failures relating to video, they look like regressions from bug 709721. The WebGL tests failing here are trying to load WebM videos. The videos never get loaded, so they time out.

I opened one of these WebM files in a new tab,
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/resources/red-green.webmvp8.webm

and got a null-nsRefPtr-deref assertion failure:

 	KernelBase.dll!_DebugBreak@0()  + 0x2 bytes	
 	xul.dll!RealBreak()  Line 422	C++
 	xul.dll!Break(const char * aMsg)  Line 521	C++
 	xul.dll!NS_DebugBreak_P(unsigned int aSeverity, const char * aStr, const char * aExpr, const char * aFile, int aLine)  Line 380 + 0xc bytes	C++
 	xul.dll!nsRefPtr<nsWebMBufferedState>::operator->()  Line 1055 + 0x23 bytes	C++
 	xul.dll!nsWebMReader::NotifyDataArrived(const char * aBuffer, unsigned int aLength, unsigned int aOffset)  Line 806 + 0x1a bytes	C++
>	xul.dll!nsBuiltinDecoderStateMachine::NotifyDataArrived(const char * aBuffer, unsigned int aLength, unsigned int aOffset)  Line 229	C++
 	xul.dll!nsBuiltinDecoder::NotifyDataArrived(const char * aBuffer, unsigned int aLength, unsigned int aOffset)  Line 511	C++
 	xul.dll!nsMediaChannelStream::CopySegmentToCache(nsIInputStream * aInStream, void * aClosure, const char * aFromSegment, unsigned int aToOffset, unsigned int aCount, unsigned int * aWriteCount)  Line 381	C++
 	xul.dll!nsInputStreamTee::WriteSegmentFun(nsIInputStream * in, void * closure, const char * fromSegment, unsigned int offset, unsigned int count, unsigned int * writeCount)  Line 223 + 0x23 bytes	C++
 	xul.dll!nsPipeInputStream::ReadSegments(unsigned int (nsIInputStream *, void *, const char *, unsigned int, unsigned int, unsigned int *)* writer, void * closure, unsigned int count, unsigned int * readCount)  Line 799 + 0x1d bytes	C++
 	xul.dll!nsInputStreamTee::ReadSegments(unsigned int (nsIInputStream *, void *, const char *, unsigned int, unsigned int, unsigned int *)* writer, void * closure, unsigned int count, unsigned int * bytesRead)  Line 277	C++
 	xul.dll!nsMediaChannelStream::OnDataAvailable(nsIRequest * aRequest, nsIInputStream * aStream, unsigned int aCount)  Line 411 + 0x1f bytes	C++
 	xul.dll!nsMediaChannelStream::Listener::OnDataAvailable(nsIRequest * aRequest, nsISupports * aContext, nsIInputStream * aStream, unsigned int aOffset, unsigned int aCount)  Line 128	C++
 	xul.dll!mozilla::dom::MediaDocumentStreamListener::OnDataAvailable(nsIRequest * request, nsISupports * ctxt, nsIInputStream * inStr, unsigned int sourceOffset, unsigned int count)  Line 117 + 0x30 bytes	C++
 	xul.dll!nsDocumentOpenInfo::OnDataAvailable(nsIRequest * request, nsISupports * aCtxt, nsIInputStream * inStr, unsigned int sourceOffset, unsigned int count)  Line 322 + 0x30 bytes	C++
 	xul.dll!nsStreamListenerTee::OnDataAvailable(nsIRequest * request, nsISupports * context, nsIInputStream * input, unsigned int offset, unsigned int count)  Line 111 + 0x35 bytes	C++
 	xul.dll!nsHttpChannel::OnDataAvailable(nsIRequest * request, nsISupports * ctxt, nsIInputStream * input, unsigned int offset, unsigned int count)  Line 4418 + 0x60 bytes	C++
 	xul.dll!nsInputStreamPump::OnStateTransfer()  Line 512 + 0x40 bytes	C++
 	xul.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream)  Line 402 + 0xb bytes	C++
 	xul.dll!nsInputStreamReadyEvent::Run()  Line 115	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait, bool * result)  Line 625 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent_P(nsIThread * thread, bool mayWait)  Line 245 + 0x17 bytes	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate)  Line 110 + 0xe bytes	C++
 	xul.dll!MessageLoop::RunInternal()  Line 209	C++
 	xul.dll!MessageLoop::RunHandler()  Line 202	C++
 	xul.dll!MessageLoop::Run()  Line 176	C++
 	xul.dll!nsBaseAppShell::Run()  Line 191	C++
 	xul.dll!nsAppShell::Run()  Line 258 + 0x9 bytes	C++
 	xul.dll!nsAppStartup::Run()  Line 220 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc, char * * argv, const nsXREAppData * aAppData)  Line 3525 + 0x25 bytes	C++
 	firefox.exe!do_main(const char * exePath, int argc, char * * argv)  Line 201 + 0x13 bytes	C++
 	firefox.exe!NS_internal_main(int argc, char * * argv)  Line 287 + 0x14 bytes	C++
 	firefox.exe!wmain(int argc, wchar_t * * argv)  Line 107 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 552 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 371	C
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes
Comment 15 Benoit Jacob [:bjacob] (mostly away) 2011-12-14 14:28:03 PST
It's consistently reproducible, just apply this patch and the one from bug 709721 (actually only bug 709721 should be enough but i didn't check) and load in a new tab the video mentioned in previous comment.
Comment 16 :Ehsan Akhgari 2011-12-14 15:15:32 PST
What's happening is that inside nsWebMReader::Init, vpx_codec_dec_init fails.  The reason is that it tries to access vpx_codec_vp8_dx_algo, which is a private variable inside gkmedias.dll.  And apparently something is preventing libxul and gkmedias.dll to see the same variable.  Accessing that variable through the accessor function fixes things.  I have a patch for that.
Comment 17 :Ehsan Akhgari 2011-12-14 15:22:11 PST
Created attachment 581810 [details] [diff] [review]
part 2: fix the variable access
Comment 18 Chris Pearce (:cpearce) 2011-12-14 15:29:38 PST
Comment on attachment 581810 [details] [diff] [review]
part 2: fix the variable access

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

We should land/track this in bug 709721. ;)
Comment 19 :Ehsan Akhgari 2011-12-14 15:51:42 PST
(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #18)
> Comment on attachment 581810 [details] [diff] [review]
> part 2: fix the variable access
> 
> Review of attachment 581810 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> We should land/track this in bug 709721. ;)

Yeah, sorry, will adjust the commit message before landing.
Comment 20 Mozilla RelEng Bot 2011-12-14 19:30:39 PST
Try run for 5026656c54b6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5026656c54b6
Results (out of 268 total builds):
    success: 236
    warnings: 32
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-5026656c54b6
Comment 21 Mike Hommey [:glandium] 2011-12-14 21:31:38 PST
Created attachment 581868 [details] [diff] [review]
Slice out the ANGLE compiler from libxul on Windows

Refreshed user info, in case someone else lands the patch.
Comment 22 Mike Hommey [:glandium] 2011-12-14 22:14:28 PST
http://hg.mozilla.org/mozilla-central/rev/beac16509534
Comment 23 Mozilla RelEng Bot 2011-12-15 00:10:45 PST
Try run for 5026656c54b6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5026656c54b6
Results (out of 270 total builds):
    success: 238
    warnings: 32
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-5026656c54b6

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