Reviewing Ember and wondering about performance issue

Performance
4

Okay, I’ve figured out the least-invasive change to fix the performance issue. HUGE thank-you to @jthoburn for his example code and extensive time on Twitter to talk through the issues.

The symptoms

The app has a spreadsheet-like table that recalculates every time the user presses a key. According to ember-inspector, each row of the table was taking about 10ms to render, and each cell took about 1ms. With 41 rows and 287 cells, this resulted in a total delay of 500-600ms per keystroke. I wanted to get down to less than 100ms, and ideally less than 10ms.

What didn’t work

I had @jthoburn’s solution, but I also wanted to try several other options that were suggested on Twitter.

Using production version of Ember

Switching the code to use ember.min.js cut render times in half, to about 200-300ms, but it didn’t address the root problem. The delay was still too noticeable.

Using one-way binding

Modifying the table components to use one-way binding had no effect on render times. I’m not 100% sure I did this right, though. Here’s the code I used (I also did the same thing in stock-market-row and stock-market-cell):

Before:

    // stock-market-table component
    module.exports = Ember.Component.extend({
        years: function() {
            // ...
        }.property("value")
    });

After:

    // stock-market-table component
    module.exports = Ember.Component.extend({
        years: function() {
            // ...
        }.property("_value"),

        value: "",
        _value: Ember.computed.oneWay("value")
    });

Using readOnly()

Adding .readOnly() to the table components’ computed properties had no effect on render times. I didn’t expect it to, but this was something @jthoburn did in his example, so I wanted to check it.

What worked:

Using @jthoburn’s code as a starting point, I was able to narrow the code down to the least change necessary to fix the performance problem. After this change, the rendering time was <1ms, with occasional spikes to ~30ms.

The solution was to reuse the object array that the table rows were based on. My original code re-created this array every time. The new code modifies the array using Ember’s MutableArray extensions.

Before:

    // stock-market-table component
    module.exports = Ember.Component.extend({
        years: function() {
            // projection is a StockMarketProjection instance, a domain object
            var projection = this.get("value");
            var rows = [];
            for (var i = 0; i < (projection.numberOfYears()); i++) {
                rows.push(projection.getYearOffset(i));
            }
            return rows;
        }.property("value")
    });

<!-- stock-market-table template -->
<table class="stockmarket">
    <thead> <!-- ... --> </thead>
    <tbody>
    {{#each years as |year|}}
          {{stock-market-row value=year}}
    {{/each}}
    </tbody>
</table>

After:

    // stock-market-table component
    module.exports = Ember.Component.extend({
        years: function() {
            // projection is a StockMarketProjection instance, a domain object
            var projection = this.get("value");
            
            var current;
            var rows = this.get("_projection");
            var numYears = projection.numberOfYears();
            for (var i = 0; i < ( numYears); i++) {
                current = rows.objectAt(i);
                if (!current) {
                    current = {};
                    rows.insertAt(i, current);
                }
                Ember.set(current, "perfOptimization", projection.getYearOffset(i));
            }
            if (numYears < rows.length) rows.removeAt(numYears - 1, rows.length - numYears);
            return rows;
        }.property("value"),

        _projection: []
    });

<!-- stock-market-table template -->
<table class="stockmarket">
    <thead> <!-- ... --> </thead>
    <tbody>
    {{#each years as |year|}}
          {{stock-market-row value=year.perfOptimization}}
    {{/each}}
    </tbody>
</table>

This was the only change needed. Note that I’ve fixed a bug that was present in @jthoburn’s code: if the number of years in the projection decreases, the extra rows wouldn’t be removed from the table. (In fairness to @jthoburn, the UI doesn’t allow that to happen.)

Overall, I’m pretty happy that the impact was so small. The code’s a bit fiddly and error-prone, as performance code tends to be, but it’s nicely localized to the one component that caused the problem.

You can see the full diff here.

Open questions:

  • Why does this work?
  • Is there an even smaller change that would also work?
  • Would Glimmer have fixed it automatically?

Edits: formatting, removed left-over console.log

2 Likes