Reviewing Ember and wondering about performance issue

Hi everyone,

I’m reviewing Ember.js for Let’s Code JavaScript’s “Unconventional Reviews” series (for example: AngularJS review). In the review, I’m implementing a stock market projection app that recalculates the projection on every keystroke.

The AngularJS and React versions handled the load without any problem. The EmberJS implementation, however, takes about half a second per keystroke.

Is this normal? The projection is rendered in a table that’s 41 rows long (and 7 columns wide). I’d heard that Ember is slow to render long lists, but I didn’t think this qualified.

Would anybody mind taking a look at the code and telling me if I’m doing something silly? If not, can you point me in the direction of current Ember performance documentation?

The code is on GitHub. The Ember-related code is in src/client/ui, except for startup code in src/client. I’m using Ember 1.10. (I tried upgrading to the beta and canary versions, but they threw exceptions.)

To run the app, download the code and run ./jake.sh run from the project’s root directory, or jake run if you’re on Windows.

You might also be interested in comparing the code to the Angular version or React version. I can’t post the links, but they’re on my github.

Thanks for your help! I’m interested in following up with a broader conversation about Ember performance, so I hope you’ll forgive my posting this here rather than on Stack Overflow.

Thanks to @Runspired, @mrmcdowal, and Tom Dale (can’t link, dratted new user limit) for their help on Twitter. (I’m ‘jamesshore’ on Twitter.) I’ve gotten some feedback that the app’s structure is a turn-off for people interested in helping; sorry about that! A big part of the review is to see how well Ember deals with things outside its comfort zone. That includes people who have existing codebases and builds, and thus don’t want to use ember-cli.

The application consists of six components; that’s it. All six components are in src/client/ui alongside their HTMLBars templates.

The app is self-contained and easy to run. If you have Node 0.10.x or higher installed, you can run it this way, no additional dependencies needed:

git clone https://github.com/jamesshore/lab16_emberjs.git
cd lab16_emberjs
./jake.sh run    # or ./watch.sh run

If you run the app, you’ll see a configuration area and a table. The configuration area is rendered by the configuration_panel component (configuration_panel.js and configuration_panel.hbs), which contains three configuration_field components.

The table is rendered by stock_market_table, stock_market_row, and stock_market_cell.

A few questions:

  1. How do people diagnose performance problems in Ember apps? I have ember-inspector installed, but it’s not clear how I would use it in this case.

  2. The table is probably the bottleneck; reducing it down to five rows eliminates the performance problem. @Runspired suggested using one-way binding. I don’t understand how to use it with a component that receives a value on a property, though. How would I change e.g., the StockMarketTable to use a one-way binding for its value property? Or is that a nonsensical question?

Thanks for helping out a newbie :smile:

PS: Switching out ember.debug.js for ember.min.js doesn’t fix the performance problems. It’s better, but not enough. That was the first thing I tried before posting. :smile:

I branched and created an ember-cli version to better analyze the performance issue and give you a solid answer. That branch is here: GitHub - runspired/lab16_emberjs at ember-cli-version

It serves as a relatively good example of a performance trap using {{#each}} that a lot of ember beginners fall into. I’d argue there should be docs letting you know how to avoid this pitfall, but the upcoming Glimmer diffing will render most of this performance pitfall mute, and if I get MagicCollectionView finished, that will solve the other portion of this as well.

##Rerendering Stats for {{#each}}

  • original code: ~380ms
  • modified code: <1ms
  • modified code when images are inserted: ~10ms (could be made faster, the image insertion method here is pretty dirty)

Eesh, that’s a big difference. What went wrong?

Before my mods, each keystroke completely rebuilt the array, so my first step was to ensure that the underlying array instance wasn’t destroyed. Most of the time, that’s all that’s needed to ensure solid performance of the {{#each}} helper.

This change helped, but it only reduced the average by ~15ms. Ouch, not the silver bullet I was hoping for. The reason was fairly clear though, I was clearing the old array using array.length = 0 and inserting brand new rows with the new data. Your rows change every time, so each of the list items will need to be re-rendered anyway.

You might already be able to guess where this is going, but the next step was to re-use the same object instances for rows. This prevents Ember from needing to teardown the component and it’s HTML and generate a new one. And yes, before it needed to build both a new component and new DOM, so your performance hit is a double whammy.

I moved what had been a series of computed properties on your table-row component to being performed by a helper function in your route controller, and passed the existing object for that row index into the helper to reuse it.

At the end of the day, there is no substitute for a good algorithm, but what you bumped into here is a very widely known performance issue for Ember, which (awesomely) will largely disappear in ~12 weeks time without everyone needing to learn to optimize their {{#each}} helpers in this way.

###Glimmer Preview

###One of many demonstrations of this particular failure

As an aside, before you use Florence’s talk to judge Ember’s performance to harshly, it should be noted, that in Ryan Florence’s demo code, he did not try to optimize his rows in the manner I show here, nor did his code make use of Ember.run.schedule('afterRender') which would have helped him a lot in this use case, and he used a version with a known performance regression, and he simply doesn’t focus the window to make the claim that ember doesn’t even get to the mouseovers. In short, there’s a lot of issues with that talk, but the main premise does hold, it’s easy to bump into this problem.

5 Likes

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

I ran into similar performance issues and this post helped a lot. I extracted my implementation into an Ember addon ember-cli-fast-table. Hope it helps people that are still in pre-Glimmer builds.