User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release) Build ID: 20130803215302 Steps to reproduce: Animation drawing performance of fennec including central build from 2/9/2013 has dramatically worse animation drawing performance on android x86 devices (testing on a sandybridge i7) compared to the default android browser (based on webkit). We are running our own builds of AOSP (from android-x86.org). I believe the iscroll library (http://cubiq.org/iscroll-4) is using CSS property animation in the demo content I have posted a link too below. Actual results: I have been testing with this demo content: http://manichord.com/opensign/ffdebug/ffscroll.html Fennec draws the automatic transistions *very* choppily where as the default browser draws at an acceptably high enough framerate. Expected results: Fennec should be drawing at roughly the same rate as the (very old) version of webkit that is used by the default browser.

This seems unrelated to x86; I get the same terrible performance on scrolling that screensaver on both an ARM and x86 based device I have here. Reproducible on my Motorola XT890 (Android 4.2) and my Samsung Galaxy SIV (Android 4.3). In that demo what I am seeing is not overly bad, I just am seeing black screen tearing when I swipe between the cats and dogs. I'm not sure if this best belongs in a component and is otherwise already known that transitions and transforms suck in general.

Aaron thanks for taking a look at this so quickly. Sorry I should have mentioned that the test content has a slideshow mode that kicks in after a few seconds and when watching that, the drawing performance and lost frames are most obvious, especially in the ease in and ease out parts of the slide animation. Also I've tested this here on a couple of arm tablets, one with a low-powered amlogic 8276mx Soc (1024x768 screen) as well as on a Nexus10 and I don't see this poor drawing performance on either of those. I should also add that again I'm testing this on small x86 PC-type device using a 3rd gen core i7 displaying to a large 40' 1080p LCD, so the dropped frames in the animation are probably more visible than on smaller screens. I'll do a profiler log on the Nexus10 with the same content and add it here is case it helps show up what might be causing this.

Apologies I got it wrong. I've retested again on the Nexus10 and Aaron you're right, the performance is pretty much the same as on x86. I'll upload the profiler capture I've got from running a 24a1 nightly from 10 June as this seems to show a very specific points in the engine as being where almost all the time is being spent.

to help compare apples with apples, this is the profiler data running the content on a nexus10 with nightly (26a1) from 2013-09-03

this profiler data is for the same content running on a ubuntu 12.04 i7 2nd gen laptop with FF nightly from 2013-09-05

Having a look through the profiling data, it looks like fennec and firefox (on ubuntu) takes quite different code paths for drawing. I can see that fennec uses ClientLayerManager and desktop ubuntu use BasicLayerManager. So I'm wondering what is the difference between BasicLayerManager and ClientLayerManager and could this have something to do with the drawing performance? In the profile data, there seem to be a far higher number of calls into ClientLayerManager. Does that sound right?

maks, fennec is using hardware compositing, where as desktop is using software compositing, hence the different code paths. BenWa, any thoughts here? qawanted to get a regression range.

Brad of course, that makes perfect sense. For regression, we've tested back to a nightly central build from 10 June and were seeing this still. I'm happy to try going back through older builds to get the earliest build where this is happening.

I've tested back through nightlies: fennec-20.0a1.multi.android-arm.apk (from 3rd of Feb 2013) fennec-18.0a1.multi.android-arm.apk (from 1st of Sept 2012) and still this issue, so perhaps this isn't a regression - could it be no one has noticed/reported this before now?

Ok, at mfinkles suggestion I tried applying the latest patch from https://bugzilla.mozilla.org/show_bug.cgi?id=845874 but unfortunately it seems to not make a difference. Looking again at the profiler output it seems a a large chunk of time it being spent in gfxContext::Fill and simliar number of BasicTiledLayerBuffer::ValidateTile which I'm not sure is to be expected ?? as the demo is of large images being animated (scrolled left) across the browser - "slideshow" style.

(In reply to maks from comment #12) > Looking again at the profiler output it seems a a large chunk of time it > being spent in gfxContext::Fill and simliar number of > BasicTiledLayerBuffer::ValidateTile > which I'm not sure is to be expected ?? as the demo is of large images being > animated (scrolled left) across the browser - "slideshow" style. That's expected since we're drawing using tiles. Try profiling with 'Breakpad' check to attempt to unwind the source. This might tell us why gfxContext::Fill is slow.

Benoit thanks for looking at this. I want to try profiling with Breakpad - I guess that means profiling the native stack not just the PROFILER_LABEL() points in the code, but I can't seem to find any instructions on how to do that for fennec. I looked here: https://developer.mozilla.org/en-US/docs/Performance/Profiling_with_the_Built-in_Profiler but the section "Profiling Firefox mobile" doesn't seem to be complete as I dont understand the "Select target 'Mobile USB'" part and everythign else in that section seems to be the same as the steps I followed to use the "psuedo stack" profiler ?

Benoit Girard (:BenWa) looking at gfxContext::Fill ... void gfxContext::Fill() { PROFILER_LABEL("gfxContext", "Fill"); if (mCairo) { cairo_fill_preserve(mCairo); } else { FillAzure(1.0f); } } seems the only thing its doing is calling cairo_fill_preserve(). I've been trying to learn how the code works from that point on and as far as I can tell, cairo would then render a bitmap and then ship it off to the Java compositor code to use androids OpenGLES to then draw it onscreen. I'm guessing that happens in LayerRenderer.java in the drawForeground(). That's as far as I have got so far, perhaps it would be worthwhile my adding some timing measurement code in there to try to see if its the bottleneck for some reason or am I barking up the wrong tree?

Since cairo is a statefull API it's hard to know exactly what ::Fill() will do without knowing what the active path(s) were. When you're connected to profiling, stop the profiler and check 'Breakpad', start the profiler and record your trace. We only draw the background & shadow with Java code, the page contents are all composited from gfx/layers/*.

Benoit thanks again for your help. I've uploaded the full profiler trace: http://people.mozilla.org/~bgirard/cleopatra/#report=16311253e5a8e996ac4e7734985e55f44197a203 From havign a quick look at it it seems to be all in bits_image_fetch_biliear_affine_pad_x8r8g8b8

oops, that whould be "bits_image_fetch_bilinear_affine_pad_x8r8g8b8", being called from gfxContext::Fill() but I can only find in the src a bits_image_fetch_bilinear_affine function?

ok for anyone elses future reference: that function is actually created by the MAKE_FETCHERS macro in gfx/cairo/libpixman/src/pixman-bits-image.c

Benoit, one thing I noticed looking at the traces was that in the case of PaintThebesSingleBufferDraw() it ended up calling down to bits_image_fetch_bilinear_affine_pad_x8r8g8b8 but for PaintThebesUpdate() it was: BasicTiledLayerBuffer::PaintThebesUpdate |->BasicTiledLayerBuffer::ValidateTile |->gfxContext::Fill |->pixman_composite_src_8888_8888_asm_neon This along with your comment about bits_image_fetch_bilinear_affine being the slowest got me thinking, why is not using any Neon simd to make things faster. This line of thinking lead me to find: https://bugzilla.mozilla.org/show_bug.cgi?id=628324 which is where scaling perf got an improvement and lead me to think this case, translation might also benefit from simliar move to simd implemented functions. That then lead me to: http://www.mail-archive.com/pixman@lists.freedesktop.org/msg00921.html where Siarhei Siamashka gives a simple patch to replace bilinear_interpolation (which bits_image_fetch_bilinear_affine calls towards its end) with a Neon based alternative. He suggests there that this is not the right place to do that and infact better optimisations should be done in the calling functions, eg. in bits_image_fetch_bilinear_affine BUT he goes on to say that it would give around a 2x improvement in speed which hes thinks isn't worht it but I ssupect in this case would still make a significant difference if that is the bottleneck in the bits_image_fetch_bilinear_affine function. Unfortunately this is where I got stuck, as quickly trying to use Siarhei's assembly "hacked" version of bilinear_interpolation won't compile for me and I get errors: {standard input}: Assembler messages: 0:27.48 {standard input}:9756: Error: selected processor does not support Thumb mode `vshll.u8 q1,d0,#8' 0:27.48 {standard input}:9757: Error: selected processor does not support Thumb mode `vdup.u8 d7,sl' ... even though I see in the gcc line that it has both: "-DUSE_ARM_NEON" and -march=armv7-a -mthumb Or perhaps the problem is that Siarhei's assembly is thumb and firefox only uses arm instuctions? I tried looking in: gfx/cairo/libpixman/src/pixman-arm-neon-asm.S gfx/cairo/libpixman/src/pixman-arm-neon-asm-bilinear.S and it looked like they were using thumb (I'm not at all familiar with arm asm so I could be completely wrong) and then again I'm not sure if those are even actually compiled and linked into fennec? Anyways hopefully this is useful and I'm not off on a wild goose chase...

Update: Jeff Muizelaar [:jrmuizel] on irc suggested I use jimDB to found out why the slow path is being taken by getting the parameter values that were being passed to the caller of bits_image_fetch_bilinear_affine_pad_x8r8g8b8 and further up the call stack. But I ran into a known bug in android 4.3 (https://code.google.com/p/android/issues/detail?id=58373) which prevented me being able to use gdb on my non-rooted nexus10. So I switched to using a different tablet running ics which uses a Allwinner A10 SoC with mali400 gpu but then I wasn't able to break on the required function. The reason for that turned out (after re-running the profiler with the new tablet) is that on this device, the slow path is *not* taken and in fact I can see a marked improvement in the framerate of the animation in the test content! That trace is here: http://people.mozilla.org/~bgirard/cleopatra/#report=5a5dc3cbb5a3cc9b9173ac125f720330a2eeb8d3 As you can see, in that trace, gfx:FillContent-Cairo (I renamed that PROFILER_LABEL from "gfx:FillContent" just to be sure it was cairo being called at that point in my custom build) ALWAYS calls the optimised pixman_composite_src_8888_8888_asm_neon() So I'm guessing that pixmans runtime check is for some reason on the nexus10 is switching to using the slow path function instead of the neon one, though as I mentioned in comment-20, I *do* see pixman_composite_src_8888_8888_asm_neon being called *some* of the time on the nexus10. But all this now leads me back to my *original* problem of the performance being poor on android-x86 devices, but now I think I have enough understanding of the issue to know what I need to look for on those devices so I'll do profile traces there and report back the results here.

I seem to have hit a bug with the profiler add-on while trying to profile fennec on device running android x86: https://github.com/bgirard/Gecko-Profiler-Addon/issues/56 I the meantime, would someone be able to help point me to where pixman code would do its runtime platform detection to decide whether or not it can use Neon or SSE2/3 optimised code?

It is likely that x86 doesn't have the same fast paths as there are for NEON.

For comparison, here is a link to a profile from an Android-x86 v4.3 (very recently released) build: http://people.mozilla.org/~bgirard/cleopatra/#report=c3e8dd62203f873442d57b9f817f830fb4bea147 The performance seems much improved over the poor performance we are seeing in 4.0.4. Any thoughts on what is triggering the performance boost in JB4.3? Thanks Nick

Siarhei posted a potential fix patch to the pixman maillist: http://lists.freedesktop.org/archives/pixman/2013-October/003019.html and I've attempted to try it out with a recent version of mozilla-central by first updating firefoxs embedded pixman to v 0.30.2 using the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=870258 and then tweaking pixman-see2.c by adding the dependencies required by Siarhei's patch. Unfortunately I don't see any improvement in looking at the automated scrolling of my test content (http://manichord.com/opensign/ffdebug/ffscroll.html) but then again I'm not sure if I've correctly applied Siarhei' patch as I *do* see a lot of compiler warnings when building fennec using the patch. I'll attach the patch that I've used, I'm hoping someone like Jeff could have a look at it to see if I've done anything wrong that might be negating and stopping Siarhei's new fast path from working ??

After some advice from Siarhei on irc to check that SSE2 was being enabled, I've been doing some more debugging on this and have made a little bit more progress. As mentioned before, I'm using Firefox for Android for x86 and having applied the patch from bugzilla to bring firefoxes bundled version of pixman upto 0.30.2 and then applying Siarhei's patch that adds scaled_bilinear_scanline_sse2_x888_8888_SRC() I Added logging output in pixman-x86.c to: #ifdef USE_SSE2 if (!_pixman_disabled ("sse2") && have_feature (SSE2_BITS)) { imp = _pixman_implementation_create_sse2 (imp); __android_log_print(ANDROID_LOG_DEBUG,"MAKS", "using SSE2 impl"); } #endif and that gives me log output that confirms SSE is being detected and enabled on my current test device (I've switched to testing on N270 netbook for now). But I also see that scaled_bilinear_scanline_sse2_x888_8888_SRC() is never called by adding log output to its start: static force_inline void scaled_bilinear_scanline_sse2_x888_8888_SRC (uint32_t * dst, const uint32_t * mask, const uint32_t * src_top, const uint32_t * src_bottom, int32_t w, int wt, int wb, pixman_fixed_t vx_, pixman_fixed_t unit_x_, pixman_fixed_t max_vx, pixman_bool_t zero_src) { __android_log_print(ANDROID_LOG_DEBUG,"MAKS", "scaled_bilinear_scanline_sse2_x888_8888_SRC"); and also strangely (compared to profiler traces for arm devices) I'm *not* seeing bits_image_fetch_bilinear_affine being called either: static force_inline void bits_image_fetch_bilinear_affine (pixman_image_t * image, int offset, int line, int width, uint32_t * buffer, const uint32_t * mask, convert_pixel_t convert_pixel, pixman_format_code_t format, pixman_repeat_t repeat_mode) { __android_log_print(ANDROID_LOG_DEBUG,"MAKS", "bilinear_fetch"); I also added logging to the lookup function: void _pixman_implementation_lookup_composite (pixman_implementation_t *toplevel, pixman_op_t op, pixman_format_code_t src_format, uint32_t src_flags, pixman_format_code_t mask_format, uint32_t mask_flags, pixman_format_code_t dest_format, uint32_t dest_flags, pixman_implementation_t **out_imp, pixman_composite_func_t *out_func) { __android_log_print(ANDROID_LOG_DEBUG,"MAKS", "lookup composite op:%d src:%d flags: %d", op, src_format, src_flags); and this gives results while running my test html content as 10-09 05:41:24.990 4507 4535 D MAKS : lookup composite op:1 src:537004168 flags: 42461943 10-09 05:41:24.990 4507 4535 D MAKS : lookup composite op:1 src:537036936 flags: 42420863 10-09 05:41:24.990 4507 4535 D MAKS : lookup composite op:1 src:537004168 flags: 42461943 10-09 05:41:25.000 4507 4535 D MAKS : lookup composite op:1 src:537036936 flags: 42420863 10-09 05:41:25.000 4507 4535 D MAKS : lookup composite op:1 src:537004168 flags: 42461943 10-09 05:41:25.470 4507 4535 D MAKS : lookup composite op:1 src:537004168 flags: 42429183 repeated (alot) though I'm not sure if thats helpful and I wasn't sure if I could use the out_func function pointer to figure out which function was being returned? Any further pointers on where to look most appreciated. PS. also posted this on the pixman maillist: http://lists.freedesktop.org/archives/pixman/2013-October/thread.html#3046

Siarhei suggested on the pixman list that its essential to get profiler traces to go forward with this. Given that the SPS profiler can't use the native stack unwinding on x86 like it can on arm, I've instead tried to add call to PROFILER_LABEL at some key points in the pixman code. Because pixman is C, I've tried to do this by adding a C++ wrapper like so... gfx/cairo/libpixman/src/profiler-wrapper.h: #ifdef __cplusplus extern "C" { #endif void mks_profile(); #ifdef __cplusplus } /* closing brace for extern "C" */ #endif and then gfx/cairo/libpixman/src/profiler-wrapper.cpp: #include <GeckoProfiler.h> #include "profiler-wrapper.h" extern "C" { void* moz_call_enter(const char *aInfo, uint32_t line) { return mozilla_sampler_call_enter(aInfo, NULL, false, line); } void moz_call_exit(void* handle) { mozilla_sampler_call_exit(handle); } void mks_profile() { PROFILER_LABEL("PIXMAN", "lookup_composite"); } } and adding the cpp file to moz.build: CPP_SOURCES += [ 'profiler-wrapper.cpp', ] BUT I can't build this and get linker errors for some reason: 1:01.74 INPUT("../../../../netwerk/srtp/src/sha1.o") 1:01.74 INPUT("../../../../netwerk/srtp/src/srtp.o") 1:01.74 INPUT("../../../../netwerk/srtp/src/stat.o") 1:01.74 INPUT("../../../../netwerk/srtp/src/ut_sim.o") 1:01.74 1:01.74 ../../../../dist/include/GeckoProfilerImpl.h:300: error: undefined reference to 'stack_key_initialized' 1:01.74 ../../../../dist/include/GeckoProfilerImpl.h:303: error: undefined reference to 'tlsPseudoStack' 1:01.74 ../../../../dist/include/GeckoProfilerImpl.h:300: error: undefined reference to 'stack_key_initialized' 1:01.74 ../../../../dist/include/GeckoProfilerImpl.h:303: error: undefined reference to 'tlsPseudoStack' 1:01.74 collect2: error: ld returned 1 exit status 1:01.74 make[5]: *** [mediaconduit_unittests] Error 1 1:01.74 make[4]: *** [libs_tier_platform] Error 2 1:01.74 make[3]: *** [tier_platform] Error 2 1:01.74 make[2]: *** [default] Error 2 1:01.74 make[1]: *** [realbuild] Error 2 Would someone know how I could get the SPS profiler label calls working in pixmans C code?

Update: I've managed to get profiling trace points into pixman in Firefox thanks to BenWa and jchens help. This basically needed a wrapper for calling from C to C++ AND importantly you need to disable tests as they cause linker errors! I did this by adding to my .mozconfig: ac_add_options --disable-tests C++ Wrapper to add to pixman: ============================================ #include <GeckoProfilerFunc.h> #include "profiler-wrapper.h" extern "C" { void* moz_call_enter(const char *aInfo, uint32_t line) { return mozilla_sampler_call_enter(aInfo, NULL, false, line); } void moz_call_exit(void* handle) { mozilla_sampler_call_exit(handle); } } ===================================================== profiler-wrapper.h: ===================================================== #ifdef __cplusplus extern "C" { #endif void* moz_call_enter(const char *aInfo, uint32_t line); void moz_call_exit(void* handle); #ifdef __cplusplus } /* closing brace for extern "C" */ #endif =========================================================

I've now been able to do a trace which includes Leaf nodes on x86. The most recent testing was on a AMD device running androidx86 ICS and with fennec patched to use pixman 0.30.2 (from bug 870258) and also the patch attached above (attachment 814023 [details] [diff] [review]). From looking at the trace, it looked like pixman in this case is not the cause of the slow done and the sse2 blt and composite functions are being used. As far as I can see, the only thing that seemed unusual to me was a large number of calls to CopyForStride in gfxImageSurface so I added some extra profiler labels there: CopyForStride(unsigned char* aDest, unsigned char* aSrc, const gfxIntSize& aSize, long aDestStride, long aSrcStride) { if (aDestStride == aSrcStride) { PROFILER_LABEL("CopyForStride", "Plain memcpy"); memcpy (aDest, aSrc, aSrcStride * aSize.height); } else { PROFILER_LABEL("CopyForStride", "loop memcpy"); int lineSize = std::min(aDestStride, aSrcStride); for (int i = 0; i < aSize.height; i++) { .... The resulting profiler trace is: http://people.mozilla.org/~bgirard/cleopatra/#report=0e9f84c0cae9c5ef0b34fd4edf40b6c7de08e887 Looking at the Sample Range of 2267,4729 shows the first of the above labels being called 248 times with a fair bit of time being spent in the memcpy which I guess is how slow it is to copy from main to gpu mem? Anyways I might sure if this is significant or perhaps some other part of the trace actually points to why this test content renders so poorly. Would be great if someone with more understanding of how the FF gfx system is supposed to work on android could have a look at this.

We hit this code when we're trying to draw to a tile that is still in use by the compositor. We detect this, and instead allocate a new tile and copy back data from the old tile to new tile. BenWa, am I correct in saying that we only need to copy back pixels that *aren't* in the dirty region? All pixels that are part of the dirty region will be redrawn (or just copied over in the mSinglePaintBuffer case), so copying them back seems like wasted work. I think we'd be able to avoid a lot of the copy costs under some workloads at least. How to implement this via the texture client in a sane way might be fun :)

Yes we don't need to copy them. But cutting around the region *could* be more expensive since we can't use memcpy as effectively.

That is very true, tuning that could be difficult too. For easy wins we could avoid the whole copy if the dirty rect entirely covers the tile rect. We could also reduce the copy down to the smallest region that we can still copy with a single memcpy call. Maksim, if you were interested in looking at this, it would be good to know how often we're drawing to the entire tile. I think after we call drawRect.Scale(), then drawRect is in the destination coordinate space. Comparing drawRect to Rect(0, 0, writeableSurface->Width(), writeableSurface->Height()) would let you know how much of the tile is being covered. If a lot of your copies are when the drawRect covers the entire tile rect, then we could skip that cost fairly easily.

Using the patch above patch from mattwoodrow to add some profiling for drawRect calls in TiledContentClient.cpp I got the following profiler trace (using test content: http://manichord.com/opensign/scroll/basic.html when moving mouse over img to cause a transition of via margin-left) http://people.mozilla.org/~bgirard/cleopatra/#report=011ac31fc4c155e2d47d36ffea640f88401503fe

With a lot of help on irc today from mattwoodrow (thanks Matt!), I tested with mattwoodrows patch above applied to current m-c using the test content as in comment35 and got this trace: http://people.mozilla.org/~bgirard/cleopatra/#report=770fa3744fd5c5c10001bad9f27f50646dbd752d Unfortunately from looking at the content, it doesn't appear to substantially improve the frame-rate and it appears from the trace that the CopyFromStride function is still being called quite alot.

(In reply to Matt Woodrow (:mattwoodrow) from comment #33) > For easy wins we could avoid the whole copy if the dirty rect entirely > covers the tile rect. We could also reduce the copy down to the smallest > region that we can still copy with a single memcpy call. Great idea! I was going to avoid the copy on a full dirty rect last year which I suspect is somewhat common but it never showed up as important in profiles. (In reply to Maksim Lin from comment #37) > Unfortunately from looking at the content, it doesn't appear to > substantially improve the frame-rate and it appears from the trace that the > CopyFromStride function is still being called quite alot. IMO I would still take this patch even if it doesn't help your particular test case.

To give another update: again with help from Matt and others (thanks!), I do have a workaround to this now by using css3 transition with separate a translate()'s applied to each of the 2 images instead of trying to translate the div containing both images. A second workaround to set perspective(1px) is then also need to work around https://bugzilla.mozilla.org/show_bug.cgi?id=927349 which causes dropped frames at the start of the animation, but the 2 together do get us to performance parity with the default android (webkit based) browser on ICS x86. For reference, a demo of this workaround fix is here: http://manichord.com/opensign/scroll/transform2c.html In private correspondence Matt has pointed out that the above works because: "If the transformed content contains more than just an image (a second image, any other sort of rendered content), then we instead first render content to a separate buffer and upload that. What gets rendered into this buffer is limited to what is actually visible on screen (for memory usage reasons)." which makes perfect sense but for any use-cases like image slideshows or animated page transitions .eg common mobile touch-UI libs which position part/most of the content off-screen and this is causing the problem we are seeing here. Would it be worth while filing a new bug report specifically for this issue? as Matt suggested that it is possible that this could be optimised to work better.

Just to add I also tried yesterdays nightly to see if the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=926706 that Chris Lord pointed us to had any effect, but I do not see any improvement with it looking at the demo content of 2 images (each sized to 1920x1080, the screen resolution of our test devices) inside a div with the div being translate()'d.

Do we have a working test case for this anywhere? http://manichord.com/opensign/ffdebug/ffscroll.html is 404 now.

James, apologies for that - I moved all my test files to a differnet subdomain, the url is now: http://test1.manichord.com/opensign/ffdebug/ffscroll.html That url should stay valid but pls feel free to copy the page and linked imgs to somewhere else if that will help.

filter on [mass-p5]

(In reply to Maksim Lin from comment #42) > James, apologies for that - I moved all my test files to a differnet > subdomain, the url is now: > http://test1.manichord.com/opensign/ffdebug/ffscroll.html This URL also doesn't work now. If you have time it would be great to bundle up the test files into a zip and attach it to this bug. That being said I don't anticipate anybody working on this bug in the near future as it has been inactive for quite a while. It would help to know if it still reproduces though. For now I'm going to move it into the Graphics component because it seems like that's where the problem lies.