-
Notifications
You must be signed in to change notification settings - Fork 2.4k
slow response when clicking item in large listview on mobile devices #4340
Comments
I can verify this issue. Tested on Ipad3 and HTC OneX I see this behaviour on my lists: Small lists: 1-2 s delay |
Here are some fiddles (to test lists outside the docs) 1.1.0: searching in 500 entries list on iPhone 4GS / iPad3 still passable; on desktop still performant, |
edit: I just tested the search performance... |
some input from JQM forum, maybe useful: bad performance for large lists best solution |
@toddparker @gseguin @frequent I did a quick profile over the weekend of lists-performance.html on my iPad. You can browse the results here: The biggest offender with 5 seconds seems to be related to a blur() call results in a pattern of calls involving fn.init and fn.removeClass ... I didn't count the number of times the pattern repeated but I would hazzard to guess that it's doing it for each item in the listview. Here's a snippet: $.fn.trigger - 5000 |
Hmmm, well git munged my indents, so you'll have to browse the data at the link I posted above.
|
@jblas: Nice work, It looks like you've come across something that needs to be optimized. What code profiler are you using? |
I'm using the profiler I wrote so we can profile directly on devices here: https://github.com/jblas/profiler/ I need to update the README.md so that it shows folks how to generate the JSON and use the viewer. |
@jblas: thanks for info and link. |
Could the culprit be line 1263 in jquery.mobile.navigation.js (line 3548 of jquery.mobile-1.1.0.js)? In the vclick handler: If I'm reading that correctly, it blurs all elements assigned the ui-btn class in the currently active page that aren't the link that was clicked. On a list of 500+ list items, that would be quite a few that are not the one that was clicked. Is that line needed for something in particular? Why not just focus the element that we know was clicked (even though, it should be focused already) rather than blur all the ones we know were not clicked. I commented it out for now in my local copy to see what happens. |
We'd have to ask @scottjehl why it's done that way. That particular chaining sequence lines up with the profile call graph that I pasted, so that's probably it. |
Thanks @jblas - I just ping'd @scottjehl to look at this. |
After running with that line commented out for that past week and observing to see if there is any change in how links react to a touch or click, I did notice some strangeness that doesn't make much sense. With touch events, it seemed to be less accurate in finding the closestLink (even though that line was above the blur line). I may have just been looking too hard for a problem and maybe I was just fat-fingering the li items or something. So, this morning I added this line as a replacement for the blur line (in case there is any reason why it was trying to blur every link except for the clicked link): $( link ).focus(); I'll do some more testing to see if there is any difference in how it reacts to touches and clicks (compared to nothing). If I still experience the fat-fingering-like reactions to touching li items, then I guess it's just my fat fingers. ;) By the way, I did notice a pretty dramatic performance improvement in touch response (no surprise there I guess). It's also worth mentioning that I've been running the jQuery Mobile Widget I built to lazy-load list items so that also helps with how quickly the linked page is loading. I have my app to a state where the performance all around is quite good actually. If anyone wants to know more about my lazyloader widget, here's the forum post where I announced it to gauge interest: http://forum.jquery.com/topic/jquery-mobile-widget-lazyloader-coming-soon |
Hey everyone. So, I did a little digging through commits to provide a little context into this line. It was added during the early releases to clean up a trail of focus/hover states that we were seeing while navigating jQM apps. Basically, focusing/tapping on another button on the page wasn't effectively blurring a button previously in focus, so button states were getting out of line. While working on the problem, I'm fairly sure we tried manually triggering focus() on the clicked link (as @dcarrith kindly suggested above), but it still wasn't effectively blurring (or perhaps more specifically, triggering Here's the commit In hindsight, the change was overly-agressive, and the performance findings above verify that. Really, we just needed to find only the buttons in the active page that have stale states and remove their classes. With our theme framework, this is a little trickier than it sounds. States are tied to letter-based themes, so the hover and focus state classes are variable. The blur and vmouseup handlers bound to these buttons keep track of that button's theme classes, so triggering blur on them was at least a clean way to clean things up. In the end, using the event triggers still might be the best approach; that said, I think we can remove the performance problem by minimizing the number of buttons that we're blurring by using a smarter selector (looking for class attributes that contain values with those prefixes). Looking at the buttonMarkup plugin, the blur event removes the following classes from a button: So those are the classes we'll need for our lookup. Something like the following might work...
In a brief console test, that seems to winnow it down to just what we need. Anyone have time to give that some analysis? |
I think the problem is still going to persist because we're still searching throughout the entire document, maybe even several times since there are multiple selectors specified. Can't we just track whatever is currently focused/active by bottlenecking things through function(s) that manage this? This would eliminate the need to search through the entire document. |
Agreed that it'll be a slow selector, so if we can improve that, that's great. But I think the big bottleneck here is the event trigger on all those elements, not necessarily the selector. Maybe we should land this first and then optimize as you suggest? On May 31, 2012, at 12:41 PM, Kin Blas wrote:
|
We're already tracking the active link with |
Can we assume that the selector will only match buttons? If so, perhaps we can call the blur handler directly, without going through jQuery's event firing mechanism. |
I guess the question for @scottjehl is if there are .ui-btn elements that are not links that can be focused/active. Also, firing the event handlers directly won't work if any of the btns use delegated handlers. I did a quick profile without scott's proposed change on an iPad 3 running iOS 5.1: and with scott's change: The old selector took 5ms to execute, the new one takes 10ms, but the overall blur call went from 5070ms down to 167ms which is a big improvement ... but, 177ms is still pretty slow in my opinion, especially on an iPad3, considering that this time doesn't include the actual loading/change of page. |
Hmm. Can someone test if @gabrielschulhof is right? If so, sounds like we can just remove this line without a regression. To test, just remove the line and navigate around on iOS and a desktop browser too, see if hover states stick as you go back and forth between pages that have been visited. |
After commenting out the line: I tested on an original iPad running iOS v5.0 and am not seeing any stuck hover states while going back and forth between normal pages, pages with data-dom-cache=true, or data-role=dialog pages. FWIW: I've also been testing in desktop Chrome on OSX, Firefox on OSX, Stock Android Browser on v2.3.7 and phonegapp webview, Stock Android Browser and phonegap webview on ICS running on Touchpad w/CM9, Blackberry tablet 2.0 stock browser and phonegap on a playbook, and last but not least, IE on Windows 7.5 on a HTC Titan. |
Sounds like @dcarrith pretty much nailed it ... |
@gabrielschulhof - you were going to land @dcarrith's suggestion, right? I'd like to land the change and live with it for a while to see if any issues shake out. |
I am developing a handheld application... using Android+Eclipse+jquerymobile+phonegap One of them, poor and slow performance in event time response (in a list of elements) I've tried all options depicted here..... I'm relatively new to jquerymobile, but this works perfectly for me , so I hope it helps you too..... How to arrange the slow performance in event clicks on a large element listIf you create a dynamic list (hundreds of Iist elements do not use the "href=" tag to bind the callback event procedure as usual.... Instead, try to do the following: Let's suppose we want to display And we want to bind an onclick callback procedure to display another screen In the page.html we have declared the container into we'll fullfil the list items: and in the javascript code var htmlStr = "
for (i=0; i<list.length;i++ ) htmlStr += " //Then we update the dom //And we make specific calls to jquerymobile to re-build the look-and-feel // If we try this, the response time for each click is of 300 ms, as expected.... I changed the javascript code like this: //declare a variable to trap the scroll event //bind a procedure to trap the scroll events .... //The code to fill the list var htmlStr = "
//supress the href //for (i=0; i <list.length;i++ ) htmlStr += " //instead, declare an id for each item for (i=0; i<list.length;i++ ) htmlStr += " //Update the dom //Make specific calls to jquerymobile to re-build the look-and-feel //bind the touchend event to each item .... This works perfectly and quick for me I suppose this is aprox the order in wich the events are triggered... touchstart I tried to gess the order of the events triggered with....
} How to speed up touch input buttonI have another issue with another feature, I need to I have in the html : ....... to display the input responsesAnd table with several buttons: 1 .... ....To achieve this I had to: $('#button1').bind ("touchstart", function (ev) { and to accelerate the refresh: //avoid navigation-bar refresh and other useless stuff...... If someone of you want more details , i gladly will explain them for you CONCLUSIONAs a general conclusion, don't use the <href=...> , nor <onclick=... > event bindings any more! I thank all you for the ideas and comments, and hope to contribute actively to the project community |
Today, I got all set up to submit a "suggested" patch to remove the reliance on window.history.back() which is called in the block of code "if( $link.is( ":jqmData(rel='back')" ) ) { ... }" in jquery.mobile.navigation.js. The call to window.history.back was contributing to the jumpiness on Android (and, I think other platforms too). Since I got all set up to run the unit tests, I went ahead and commented out the aforementioned "$( "." + $.mobile.activePageClass + " .ui-btn" ).not( link ).blur();" line and ran the unit tests. They all passed. I'd be glad to submit a pull request for that change first before submitting the main pull request for the partial rewrite of the click handler (which should only affect clicks on buttons with data-rel="back" attributes). I'll be submitting the main pull request tomorrow sometime as long as my testing goes well. |
dcarrith, I'm very glad to hear those good news, please submit the patch to arrange all these features.. Now , (perhaps with the patch this is arranged too), but I would ask you a litlle help, guys.... I've got some strange behaviour in the callbacks when When putting back the color of keys onpressed, there are sometimes the color change does not take effect The html:KEYPAD
the callbacks....//on touchstart
//on touchend
} I had thought to use a plugin for phonegap, for android input method, but there is a plugin to show the keyboard only... and is not customized... If I just could make my html + javascript code work quick and nice together with jquery mobile.... Could someone help me, please? |
I'm trying JQM listviews with |
@frequent I don't have an iOS3 device. Could you make a video of the behavior you're describing? |
@gseguin: on the road. Can you wait until tomorrow night? |
absolutely! |
Alight @scottjehl figured out the root of the issue here. In 1.0, we first scrolled to the top of the page before starting a transition. In 1.1, we wanted to avoid this jump so we now fade the page out first by default. As it turns out, devices like an iPhone seem to keep the page above your scroll position in memory when doing a transition, but not the part of the page below. So if you're on a long, complex page like the listview performance page with 500 items, clicking the first list item will work pretty well, but one halfway down will be very slow and a bottom one will crash the browser. Why? Because the browser is trying to animate a really massively tall page when scrolled down due to how it handle things. Test here: http://jquerymobile.com/test/docs/lists/lists-performance.html As a test, we added another condition to say that if you're either coming or going to a page where the scroll position is 3x the height of the device's screen, skip the transition. This makes the navigation back to it's snappy self. You'll notice we're using scroll position, not screen height here because I can navigate to a 10,000 pixel tall page fine but if I need to navigate away from that same page when scrolled down (or nav back to this with a scroll position restored), it will need way more memory to run the transition and could be slow or fail. This 3x value is just a starting point, we can use whatever criteria we want. Here is the change to line 28 in jquery.mobile.transition.js to give this a spin:
The question is how to layer this in. Do we just pick a value for this for 1.1.1 as a safely valve for performance now, then expose a config option for 1.2 to set the criteria? This is parallel to @gseguin or @johnbender - do you want to give this a look and think about how we should get this in? |
After discussing with @gseguin, we're going to land this new transitions scroll fix as an option called |
I ended up using a function: $.mobile.getMaxScrollForTransition since the screen size is not necessarily constant. |
Thanks so much @gseguin - this fix seems to get performance back to to where it should be. So good to have this one closed out with a solid fix. Is this also in 1.1-stable? Let us know if you're still seeing issues. |
@gseguin: hardware issues... video coming eventually.... |
@toddparker, indeed I cherry-picked the fix into 1.1-stable |
@frequent - are you seeing an issue with this fix? It just automatically sets the page transition to none if the page you're on or going to is scrolled down very far. |
I've tested the list performance page with iPad 1 iOS 4 and it works as fast as with 1.0.1. Good job @gseguin, indeed! |
All credit goes to @scottjehl, I just integrated. the patch. |
@toddparker @gseguin: it's fast but sort of hiccupy on iPad 1, iOS3.2 (I guess thats almost an oldtimer...). Here is the best video I managed with my phone... another oldtimer :-) |
@frequent - from what I can see, that's just the slower CSS rendering of all the gradients and rounded corners so it's not a regression. @MauriceG - wow, that 2,600 list item example is intense. I can confirm navigation works quickly when tapping the last item - maybe 3 seconds to show the result page and immediate active state on my 4S. In general, if folks are going to use really long lists they should tune their CSS to reduce the amount of gradients, shadows and roused corners to speed up the lists by overriding the styles. Flat = fast. Good to hear this seems fixed completely. |
@gseguin @scottjehl @toddparker - Is there a reason why getMaxScrollForTransition is a function rather than an global config variable like so many of the other global configs documented here: http://jquerymobile.com/demos/1.1.0/docs/api/globalconfig.html I was messing around in the code today and started thinking that getMaxScrollForTransition should be more like the rest of the globalconfig variables. So, the relevant part of the transitions code would look something like this instead: //---------------------------------------------------------------------------------------------- // generate the handlers from the above // Make our transition handler the public default. //transition handler dictionary for 3rd party transitions $.mobile.transitionFallbacks = {}; // Set the maxScrollForTransition to default if no implementation was set by user //---------------------------------------------------------------------------------------------- That way, developers can override it in the "standard way" in mobileinit (i.e. following the standard outlined in the globalconfig documentation above): For example, something like this:
Compared to what would be required now (unless I'm missing something):
I guess it doesn't really matter, but I was just curious why the getMaxScrollForTransition is being treated differently than the rest of the global configs. |
Hi @dcarrith, The reason for using a function is that the screen size can change during runtime on desktop browser or after orientation change on mobile devices. A function gives the user more flexibility: it can return a constant or a dynamic result based on the size of the screen. |
Ah, okay - that's brilliant. Thanks! |
I've read the entire thread and don't fully understand the fix. I have this exact problem (anemic list performance). How can I go about correcting it? Do I simply set getMaxScrollForTransition to something or would one of the other solutions work better? |
On mobile devices, tested on iOS5 and Android ICS/Chrome beta, using the doc's list performance test page http://jquerymobile.com/demos/1.1.0/docs/lists/lists-performance.html, if you click/tap any of the items, there is a 3-4 second delay before the button press is displayed and then another 1-2 seconds before the page transition occurs.
This is exacerbated by the size of the list, larger lists have worse performance.
It seems this wasn't much of an issue in 1.0.1
The text was updated successfully, but these errors were encountered: