airbnb / infinity

UITableViews for the web (DEPRECATED)
http://airbnb.io/infinity/
Other
2.8k stars 281 forks source link

Hang in Chrome and FFox #6

Open DeaconDesperado opened 12 years ago

DeaconDesperado commented 12 years ago

I'm currently experiencing a browser hang in both Firefox and Chrome when implementing infinity on relatively small lists.

What's strange is the hang only occurs when refreshing after opening or closing the console (Firebug or Web Inspector). Consecutive refreshes with the console either left open, or not opened, do not cause the hang (it only hangs when the state of the console is changed.)

DeaconDesperado commented 12 years ago

I should note that the hang occurs on the infinity demos as well.

reissbaker commented 12 years ago

Hm -- I can't reproduce in any browser on OSX Lion. Very tempted to call it a browser (or more specifically, a developer tool) bug, since refreshing has little to do with Javascript, and it only happens while changing state in developer tools. Do you mind sharing the OS you're using? Do you have any custom developer extensions installed?

Will attempt to reproduce on other machines in the meantime.

Let me make sure I understand the steps to reproduce:

  1. Open a console.
  2. Refresh.

and/or

  1. Open a console.
  2. Close a console.
  3. Refresh.
DeaconDesperado commented 12 years ago

Certainly! I'm on Ubuntu 12.04. The problem manifests in both the developer tools for Firefox and Chrome.

I would not be surprised if it had something to do with the way the developer tools are hooking into the scripts like you say.

The steps to reproduce are as follows.

Load the page. First time everything is good. Open a console and refresh, and the page will hang with cpu usage in the chrome task manager at approx 100%. You can kill the tab there, and then run a refresh and on my machine the tab will load correctly, now with the console open. However, if you close the console and refresh again, the same hang happens. Basically the hang occurs only after you open or close the dev tools and refresh. Repeated refreshes with no changes to the dev tools open close state will yield no problems at all.

DeaconDesperado commented 12 years ago

I've found by following the server-side request log that when the console is opened, ajax requests are sent continuously for more pages infinitely. This happens whether the page is scrolled or not, immediately after the console is opened.

edfuh commented 12 years ago

+1 the demo page crashes Chrome, Firefox on OSX Lion, and Mobile Safari too. All without a console open.

DeaconDesperado commented 12 years ago

What event hook actually fires callback that is passed for lazy parameter? For me it appears to be firing 20 times right at page load before any scroll event happens. If the console is open it fires continuously and indefinitely.

Firefox tries to stop the script at line 321... Chrome seems to allow it to repeat the ajax requests without interfering.

DeaconDesperado commented 12 years ago

Sorry to keep posting so quickly, but I've noticed now that drastically resizing the window also starts the infinite requests issue, this time regardless of the console state. I'm beginning to think its related to the window redraw, since the console changes reduces the viewport size and scroll dimensions just like resizing the window would. Sorry for the false positive about the dev tools.

reissbaker commented 12 years ago

@DeaconDesperado -- that makes a lot of sense! The resizing code is very naive, and probably is resulting in running the callbacks twice. It's strange that the infinity demo is doing any network requests, though -- the lazy load is just setting the src on the images, and setting it twice to the same value shouldn't do much. Maybe it does on some browsers in Ubuntu, though? Regardless, it's an actual bug: the pages are losing track of whether they've been loaded on resize. I'll work on a fix.

reissbaker commented 12 years ago

@edfuh: Are you running the demo on an older laptop? The demos are both quite CPU intensive due to the number of CSS3 effects on the page, and it's quite possible that that's the culprit. If the demo-off crashes as well, it's probably unfortunately your browser choking the large number of elements it loads upfront, and the expensive effects each element has. I've tested on multiple OS X Lion machines across Chrome, FF, and Safari -- in fact, I built the demo on a Lion laptop -- and can't reproduce.

DeaconDesperado commented 12 years ago

@reissbaker I should be clearer - I'm only able to track the network requests in my app I've been working infinity into on the boilerplate, since they print out on my test server running in the terminal. The source for the appended data in my situation is another call to the server, and that's why there are so many. I'm not sure what the network/ajax activity is on the demos, but I would assume the root problem is the same, since it's manifesting the same way even though the data source is different.

edfuh commented 12 years ago

@reissbaker Nope, i7 macbook pro. Less than a year old.

reissbaker commented 12 years ago

@edfuh: strange -- that's the same laptop I'm typing this on, but I can't reproduce. Does the demo-off work for you?

@DeaconDesperado: ah, ok.

reissbaker commented 12 years ago

@DeaconDesperado -- also, to answer your previous question: the lazy callback, if provided, will be called on any ListItems that are in (or close to) the viewport. The reason it's being called many times on startup is likely because there are many items in the viewport that it needs to be called on, even before a scroll event occurs.

DeaconDesperado commented 12 years ago

@reissbaker It calls on the items themselves? Maybe I'm confused... So is lazy the callback where I would load further entries to be appended when the user scrolls? My understanding from the documentation was the I could put the ajax logic in that callback to go back to the server and get the next page of JSON data to generate the new ListItems from and them add them to the view.

reissbaker commented 12 years ago

Aha, now I'm starting to understand what's happening.

The lazy callback is called on the ListItems that are onscreen (or close to it). Lazy-loading is a technique to have placeholder elements that are not fully loaded offscreen, and then load them fully once they're on. For example, at one point at Airbnb we used a plugin to lazily load images as they scrolled into the viewport; this meant that the page would have a much faster upfront load time, because most of the images were not actually in the DOM -- instead, placeholder elements that didn't load anything were. Once a user scrolled down far enough, we would load the images for them as needed.

The lazy callback is meant to encapsulate that kind of viewport-based loading behavior. However, it isn't meant to load in new ListItems once the user hits the bottom of the ListView -- it's just to load in and replace placeholder content. Loading more data is up to you; Infinity can be used for infinite feeds, but can also just be used for very long but finite ones.

Now the large number of requests makes sense -- you meant it to be called once they user hit the bottom, but Infinity was calling it on every element. And since resize events currently reset the items' loaded status, you were probably getting tons of requests on resizes.

The resize bug still should be fixed, but now I'm understanding a little better what's happening. Even if the loaded state didn't get reset on resize, you'd still be seeing more AJAX requests than you'd expect -- because lazy callback is for loading in extra content for existing elements, and not about loading in new elements to be appended to the ListView.

I'll add a new issue asking for load-more-elements callbacks, which seems like a reasonable thing for Infinity to encapsulate. Keeping this open for now because of the resize bug, but will close once that's fixed. I'll also update the documentation to be more clear about what it means by lazy-loading.

reissbaker commented 12 years ago

Yikes! Brief update. There was a separate bug that also could result in an unbounded number of resize events to be fired. This is now fixed, but I'll still leave open the other resize bug that results in lazy-loading things twice.

DeaconDesperado commented 12 years ago

I see, I had a pretty serious misunderstanding. So what's the lazy function doing in the demo? Just applying the src to the images to load them and inserting some general test data (the dog names and tags) ?

reissbaker commented 12 years ago

You got it.