ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.94k stars 13.52k forks source link

perf: ion-slides crash on iOS with memory error #3722

Closed neilgoldman closed 9 years ago

neilgoldman commented 9 years ago

Type: perf

Platform: ios 8 webview

Ionic crashes on iOS with a memory exception when you have more than just a few ion-slides in an ion-slide-box. Confirmed in both the phone's browser and 'native hybrid' apps on the device. So far I've only been able to test it on several iPhone 5S and iPhone 6 with iOS >= 8.0.0.

I've made this test to reproduce it and had most of my office run it. You can open it in Safari, or try building it 'natively'. Just tap the button to add more slides until it crashes.

http://play.ionic.io/m/229b294cf005

Most people have their browser crash at around 30 slides, or around 80 slides. But even on low-end android devices, using the stock browser, we can get into the hundreds without crashing.

The actual content of the slides doesn't seem to matter. It seems to crash around the same threshold with slides containing lots of images, or slides just containing "

foo

"

Edit: Here's an additional Ionic playground to show that it's not just ng-repeat causing the issue. It simply has 90 elements of

foo

inside the slide-box. Just tried it on my iPad Mini 3 and it perpetually crashes the page until Safari stops trying. http://play.ionic.io/m/77d031c7757b

Last edit: I think the issue is CSS transforms? Disabling transforms on .slider-slide solves it for me. http://play.ionic.io/m/0080892702e9

perrygovier commented 9 years ago

Thanks for the examples and thorough documentation

hui commented 9 years ago

+1

bis27 commented 9 years ago

Hi,

I am also getting a similar crash with ionic slide-box. I have raised some issues with a code sample and further details (http://forum.ionicframework.com/t/ionic-slide-box-crash-on-ipads/25231/1 && http://forum.ionicframework.com/t/memory-issue-with-slide-box-and-scrollable-content/10351/36).

For me the crash happens for larger screen devices like iPad 3 and iPad Air. But I think I will see similar crash if I increase larger no. of slides.

If any possible solution is available, please let all know

neilgoldman commented 9 years ago

I was able to mostly avoid the issue by optimizing the slides a bit.

This CSS rule helped a bit. Though I would make sure that it doesn't cause any other graphical glitches in your app.

.slider-slide {
    backface-visibility: hidden !important;
    -webkit-backface-visibility: hidden !important;
}

Also using ng-if to remove any slides not visible helped. mhartington has an example here:

http://codepen.io/mhartington/pen/kFhAp?editors=101

Though for me, it helped most to place the ng-if on the highest level container I could within the slide (in the above codepen, that would mean moving it from the

, and onto the tag, in our case it is an ). We haven't really noticed any negative performance impact from the directive being removed/inserted during sliding, and it's stopped the crashing at lower levels of slides (30 is the maximum in my case).

bis27 commented 9 years ago

Hi neilgoldman305,

Is the solution mentioned at https://github.com/driftyco/ionic/pull/3790 working for you?

I still see the same crash in my actual app even if I dont do rotation.

Based on your suggestion, I reduced the data in my slide-box and now I dont see crash if orientation is locked.

However, if my slide-box is full and I do a rotation, the crash still happens :(

bis27 commented 9 years ago

@perrygovier, is there any plan to look into issue #3722?

I have put a sample code at https://github.com/bis27/ionic-slide-box-crash which shows the crash on the latest ionic version v1.0.0 on iPad 3 and iPad Air as soon as we rotate the device !

In our actual product, we are creating ionic slide-boxes dynamically with 3-5 ion-slides by selecting some options from a side-menu. Each ion-slide item has either an ng-repeat or a collection-repeat or an iframe drawing a chart on canvas. Once the data set increases, the app simply crashes on rotation or when we are inside a slide-box and trying to delete the existing one and create a new one (by clicking our app's side-menu options)

Such crashes are not noted on lower screen-size devices like iPhone and iPad Mini. Hence the app behaves fine on these devices but simply crashes on iPad3/Air. So I cannot change the ionic controls for iPad app only

This fix is extemely important for our product launch. Please suggest if any workaround can be done

5uper commented 9 years ago

If I have more than 10 slides in my slidebox, I have the same crash at my ipad air.

Cordova CLI: 5.0.0 Ionic Version: 1.0.0 Ionic CLI Version: 1.4.5 Ionic App Lib Version: 0.0.22 ios-deploy version: 1.6.1 ios-sim version: 3.1.1 OS: Mac OS X Yosemite Node Version: v0.12.4 Xcode version: Xcode 6.3.2 Build version 6D2105

Is there a way to bypass the crash?

phuongpt commented 9 years ago

actually you must reduce slides will be show (current, current- 1, current +1 ) you can use directive like this:

module.directive('optimizeSlides', function () {
    return {
      link: function (scope, elem, attrs) {
        scope.$watch(function () {
          return scope.$eval(attrs.activeSlide);
        }, function (val) {
          var array = [scope.$eval(attrs.activeSlide), scope.$eval(attrs.activeSlide) + 1, scope.$eval(attrs.activeSlide) - 1];
          for (var i = 0, len = scope.$eval(attrs.optimizeSlides).length; i < len; i++) {
            if (array.indexOf(i)>-1) {
              scope.$eval(attrs.optimizeSlides)[i].show = true;
            } else {
              scope.$eval(attrs.optimizeSlides)[i].show = false;
            }
          }
        });
      }
    };
  });

and html like this

< ion-slide-box  active-slide="activeSlide"   optimize-slides="results" >
            < ion-slide ng-repeat="image in results">
             < div ng-if="image.show" >..

hope this help

5uper commented 9 years ago

@phuongpt thanks, no longer crashes on my ipad air

perrygovier commented 9 years ago

I dug in to this a bit more. There's nothing Ionic's doing specially to cause the memory error, it's just that there's too much going on because all those panes are in the active DOM. This is intentional to maximize performance, but I'd advise using approaches like @mhartington's or @phuongpt's if you run in to issues. Since this isn't really an Ionic bug, I'm going to close the issue. Thanks for the contributions, everyone.

SoaringTiger commented 9 years ago

I've found a simple way to solve it by ng-if and $index:

<ion-slide-box on-slide-changed="slideChanged(index)" show-pager="false" ng-if="Images">
    <ion-slide ng-repeat="image in Images">
        <img width="100%" ng-if="$index==currentSlide || $index==0" ng-src="{{image}}">
    </ion-slide>
</ion-slide-box>
$scope.slideChanged = function(currSlide) {
        $scope.currentSlide = currSlide;
    }
cquezpro commented 8 years ago

@phuongpt I got same crash on iPad3. < ion-slide-box active-slide="activeSlide" optimize-slides="results" > < ion-slide ng-repeat="image in results"> < div ng-if="image.show" >..

what is activeSlide here?

cquezpro commented 8 years ago

Hi what is activeSlide?

cquezpro commented 8 years ago

@SoaringTiger This is working on iPhone, but not working on ipad. I can swipe up to 2nd slide, but can't find image on 3rd slide. and slider is freezen,

hengluo commented 8 years ago

this all solution is working in the first two slide,but... I can swipe up to 2nd slide, but can't find image on 3rd slide. and slider is freezen, the same problem like sirius2013's

1981cxp commented 8 years ago

+1