Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-23447] Android: Optimize removeAllChildren()

GitHub Issuen/a
TypeImprovement
PriorityHigh
StatusOpen
ResolutionUnresolved
Affected Version/sn/a
Fix Version/sn/a
ComponentsAndroid
Labelsandroid, community, remove, speed
ReporterMichael Gangolf
AssigneeAshraf Abu
Created2016-05-27T12:04:55.000+0000
Updated2017-08-11T14:52:37.000+0000

Description

The current implementation of [removeAllChildren()](https://github.com/appcelerator/titanium_mobile/blob/415bd6c66dcc55b1a59a59574f3babd3c3a84ede/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java#L719) creates a copy of all views and removes each view in a while loop. The people at Shockoe showed in a [blog post](http://shockoe.com/development/profiling-titanium/) that it takes up to 24 seconds to remove 1000 views with
theWindow.removeAllChildren();
Changing the TiViewProxy method to use the native [ViewGroup.removeAllViews()](https://developer.android.com/reference/android/view/ViewGroup.html#removeAllViews%28%29) would bring down delay below 100ms! *Test app:* https://github.com/bpulley/titanium-profiling can be used to test the behavior. Just run the "Remove views en masse" test.
var win = Ti.UI.createWindow({
    backgroundColor: "white"
});

var view = Ti.UI.createView({
    top: 0,
    left: 0,
    right: 0,
    height: 100,
    layout: "horizontal"
});

var scrollview = Ti.UI.createScrollView({
    top: 100,
    left: 0,
    right: 0,
    height: 100,
    layout: "horizontal",
    type: "horizontal"
});

for (var i = 0; i < 100; i++) {
    var v = Ti.UI.createView({
        width: 10,
        height: 10,
        backgroundColor: "red",
        left: 10
    });
    view.add(v);
    v = null;

    var v2 = Ti.UI.createView({
        width: 10,
        height: 10,
        backgroundColor: "red",
        left: 10
    });
    scrollview.add(v2);
    v2 = null;
}

// Listview

var listView = Ti.UI.createListView({
    top: 200,
    height: 100
});
var sections = [];

var fruitSection = Ti.UI.createListSection({
    headerTitle: 'Fruits'
});
var fruitDataSet = [{
    properties: {
        title: 'Apple'
    }
}, {
    properties: {
        title: 'Banana'
    }
}, ];
fruitSection.setItems(fruitDataSet);
sections.push(fruitSection);

var vegSection = Ti.UI.createListSection({
    headerTitle: 'Vegetables'
});
var vegDataSet = [{
    properties: {
        title: 'Carrots'
    }
}, {
    properties: {
        title: 'Potatoes'
    }
}, ];
vegSection.setItems(vegDataSet);
sections.push(vegSection);

listView.sections = sections;

var fishSection = Ti.UI.createListSection({
    headerTitle: 'Fish'
});
var fishDataSet = [{
    properties: {
        title: 'Cod'
    }
}, {
    properties: {
        title: 'Haddock'
    }
}, ];
fishSection.setItems(fishDataSet);

function onOpen(e) {
    var startTime = new Date();
    console.log("View length: " + view.getChildren().length);
    view.removeAllChildren();
    console.log("View after remove length: " + view.getChildren().length);

    console.log("ScrollView length: " + scrollview.getChildren().length);
    scrollview.removeAllChildren();
    console.log("ScrollView after remove length: " + scrollview.getChildren().length);

    console.log("ListView length: " + listView.getSections().length);
    listView.removeAllChildren();
    console.log("ListView after remove length: " + listView.getSections().length);
    
    var endTime = new Date();
	var delta = endTime - startTime;
    alert('time ' + delta + 'ms');
}

win.add(listView);
win.add(view);
win.add(scrollview);
win.addEventListener("open", onOpen);
win.open();
Ti SDK 5.2.2 = 4160ms (and listview.removeAllViews() is not working) Patched Version = 333ms (listview will be empty)

Comments

  1. Michael Gangolf 2016-05-27

    I'm still testing the PR in various situations. It is working already with normal Views and ScrollViews. Both showed a big time improvement.
  2. Michael Gangolf 2016-05-27

    Added example and listview is working, too
  3. Sharif AbuDarda 2016-05-27

    Hello, I tried to test the issue. Here are my findings. Classic code provided above: 1. Android Emulator (6.0.0)- 4482ms(1st run), 3716ms(2nd run). 2. Android Device (4.4.2)- 4658ms. Alloy sample in Github 1. Android Emulator (6.0.0)- Removed 1000 views in 18860ms. 2. Android Device (4.4.2)- Removed 1000 views in 13954ms. Listview removeAllChildren() needs to be optimized. Regards, Sharif.
  4. Michael Gangolf 2016-05-27

    Those numbers are with the PR or without?
  5. Michael Gangolf 2016-06-04

    Removed the ListView part! The removeAllChildren() inside the TiListView doesn't remove the ListViewItems in the current SDKs (as they are not chlidren of the ListView), so I don't need to do it in this PR. I just added an if-part so it won't produce an error if you call it on a ListView (behaves the same as in e.g. 5.2.2.GA).
  6. Michael Gangolf 2016-06-14

    @sdarda did you test it with the patch, too? Any other notes on the behavior? It will just work on View with children, so ListView is not included (and works like before)
  7. Michael Gangolf 2016-06-27

    Using the app from the blogpost and android monitor heap: *before:* !http://migaweb.de/mem1.png! *during creation* !http://migaweb.de/mem2.png! !http://migaweb.de/mem3.png! *after deleting the views and pressing GC:* !http://migaweb.de/mem4.png!
  8. Ashraf Abu 2016-06-28

    The PR by [~michael] for this ticket is here: https://github.com/appcelerator/titanium_mobile/pull/8026
  9. Hazem Khaled 2016-12-15

    It'll be great if you release this issue with TIMOB-19482 together 6.1.0 :)

JSON Source