[TIMOB-23447] Android: Optimize removeAllChildren()
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | High |
Status | Open |
Resolution | Unresolved |
Affected Version/s | n/a |
Fix Version/s | n/a |
Components | Android |
Labels | android, community, remove, speed |
Reporter | Michael Gangolf |
Assignee | Ashraf Abu |
Created | 2016-05-27T12:04:55.000+0000 |
Updated | 2017-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)
I'm still testing the PR in various situations. It is working already with normal
Views
andScrollViews
. Both showed a big time improvement.Added example and listview is working, too
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.
Those numbers are with the PR or without?
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).@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)
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!
The PR by [~michael] for this ticket is here: https://github.com/appcelerator/titanium_mobile/pull/8026
It'll be great if you release this issue with TIMOB-19482 together 6.1.0 :)