We have this code in our post view:
const PostView = Discourse.GroupedView.extend(Ember.Evented, {
classNameBindings: ['needsModeratorClass:moderator:regular',
'selected',
'post.hidden:post-hidden',
'post.deleted:deleted',
'post.topicOwner:topic-owner',
'groupNameClass',
'post.wiki:wiki',
'whisper'],
...
Ember has this feature where it allows you to set class names for the DOM element its rendering.
In Android we render 10 posts, everywhere else 20.
This little snippet of code looks innocuous and is following the best practices.
It is ultra convenient, if I add the .deleted property on post I know that magically, somehow, Ember will go ahead and fix up the class name so you have <div class='deleted'>
The problem though is that this feature has a cost.
_applyClassNameBindings: function () {
window.counter1 = window.counter1 || 0;
var start = window.performance.now();
var classBindings = this.classNameBindings;
if (!classBindings || !classBindings.length) {
return;
}
var classNames = this.classNames;
var elem, newClass, dasherizedClass;
// Loop through all of the configured bindings. These will be either
// property names ('isUrgent') or property paths relative to the view
// ('content.isUrgent')
enumerable_utils.forEach(classBindings, function (binding) {
var boundBinding;
if (utils.isStream(binding)) {
boundBinding = binding;
} else {
boundBinding = class_name_binding.streamifyClassNameBinding(this, binding, "_view.");
}
// Variable in which the old class value is saved. The observer function
// closes over this variable, so it knows which string to remove when
// the property changes.
var oldClass;
// Set up an observer on the context. If the property changes, toggle the
// class name.
var observer = this._wrapAsScheduled(function () {
// Get the current value of the property
elem = this.$();
newClass = utils.read(boundBinding);
// If we had previously added a class to the element, remove it.
if (oldClass) {
elem.removeClass(oldClass);
// Also remove from classNames so that if the view gets rerendered,
// the class doesn't get added back to the DOM.
classNames.removeObject(oldClass);
}
// If necessary, add a new class. Make sure we keep track of it so
// it can be removed in the future.
if (newClass) {
elem.addClass(newClass);
oldClass = newClass;
} else {
oldClass = null;
}
});
// Get the class name for the property at its current value
dasherizedClass = utils.read(boundBinding);
if (dasherizedClass) {
// Ensure that it gets into the classNames array
// so it is displayed when we render.
enumerable_utils.addObject(classNames, dasherizedClass);
// Save a reference to the class name so we can remove it
// if the observer fires. Remember that this variable has
// been closed over by the observer.
oldClass = dasherizedClass;
}
utils.subscribe(boundBinding, observer, this);
// Remove className so when the view is rerendered,
// the className is added based on binding reevaluation
this.one("willClearRender", function () {
if (oldClass) {
classNames.removeObject(oldClass);
oldClass = null;
}
});
}, this);
window.counter1 += window.performance.now() - start;
}
});
On my i7 4770K Win 10 box, the cost of this convenience amortized across displaying 20 posts runs from 5ms to 15ms
On my Nexus 7 the same feature costs 35ms to 50ms. Considering it is only rendering half the posts the cost of the convenience on Nexus 7 is 14x slower than my desktop for this particular abstraction.
This is just the upfront cost, then go and add hidden teardown cost, GC cost and so on which on Android is severe. A lot of the slowness in Android is actually due to teardown of previous page (on desktop we usually beat the XHR but this is almost never the case on Android)
This specific example is just an illustration of the endemic issue. In React and some other frameworks you just render and don’t carry around the luggage of “binding”, instead you just rerender when needed. This approach means that you don’t need bookeeping, the bookeeping is the feature that is killing android perf.
I am confident Chrome will be able to catch up to Safari and bridge the 40% performance gap we are currently observing. Having Android be 40% faster will be awesome, and we don’t even need to do anything but wait to get that.
However, all we can hope for is a 40-60% perf gain. Considering it takes seconds to render a topic on a Nexus 7 and sometimes even 10 seconds to render a complex page like the categories page, I am not convinced this is quite enough.
Thing is, we know when a post is deleted (we hit delete) … a post becomes a whisper (never) … turns into a wiki (wiki button clicked) and so on. We can afford some extra code for these cases, especially for huge perf gains. We really only care about 2 pages when it comes to optimising heavily (topic/topic list)
The only approach that will be fast enough for Android devices in the next 2-3 years is going to be unbound renders with triggered refreshes for our topic and topic list page.
Ember can probably get 20% faster, Chrome will probably get 40% faster, but a React style unbound render is going to be an order of magnitude faster, it’s a totally different ballpark.
I am confident we can keep one codebase and be maintainable and fast enough, we just need to work closely with @tomdale and @wycats to unlock some of this unbound goodness in Ember (which is already unlocked quite a bit for Fast Boot) and we can make Discourse fast even on Android.
