Imba todomvc benchmark is cheating :(

[imba]

Imba todomvc benchmark is cheating :(

[adamhaile]

Hi Sindre -- Sorry for the slow reply. We're under a crunch at work right now.

Also I, too, need to apologize for coming across as unduly harsh on HN. "Cheat" and "sneaky" have a usage in benchmarks, but they also carry a moral accusation. I meant much more the former than the latter, but that wasn't clear enough.

I've been looking more at Imba, and I continue to find cool stuff. I've been playing around with my own framework, and there's a lot of food for thought in Imba. I also understand better why you were frustrated by my removal of caching altogether. You've gone to great lengths with Imba to separate out the minimum work needed to do an update, and removing all caching defeats those efforts. So no caching isn't "the Imba way." At the same time, tuning the caching strategy to the benchmark doesn't seem totally fair either. It means that the results aren't representative of behavior in general.

My take on what would make the benchmark more "fair":

• only hold onto the current live todo views. You have to admit that holding onto the rendered todo views when the original todo has been removed from your model (which usually means "gone from the universe") seems like it's tuned to the particular behavior of your benchmark, where such a todo suddenly reappears. This is a more conservative strategy than what you've got right now (hold onto max number of todos), but I think it makes a greater case for Imba's speed. I'm much more impressed by someone saying "we're 8x faster with a conservative implementation" than "we can tune to the benchmark and get 60x."
• there should be a benchmark for rendering genuinely new todos. DOM creation is an important part of a framework and should be tested. (Also, you're really fast here, though not 60x, so you might as well show that.)
• if you're going to use Imba's performance strategies (caching and minimum update work), then you should be fair and use React's as well (shouldComponentUpdate). I don't follow the argument that you had to slow down React to be fair to Mithril (?), especially when Mithril has its analogue, { subtree: 'retain' }.

I did these changes with my clone of the benchmark and came up with Imba being about 7x faster than React (Chrome/win7). I think that's a) awesome, and b) probably more indicative of real world performance.

More generally, I have a couple comments on the benchmark as a whole:

• I'm not sure I follow the value in a "full update" benchmark. We usually need two things: render the initial full view, and update as necessary based on changes. What's the need for testing full update speed?
• I liked that the original todomvc benchmark tested render and event-handling time as well. Different frameworks interact with the DOM more or less efficiently. The React folks claim that they do some expensive JS calcs (like Levenshtein distance array mutations) in order to minimize DOM mutations, as that produces an overall faster experience. You're effectively penalizing them for that here.

Again, I think Imba is very cool, even if I'm critical of this benchmark and its relevance to Imba's speed in general usage.

(If you're curious: you beat my framework by 2x. I'm faster on rename and toggle, but you're 3x faster on reorder. My algorithm for reconciling a node's children is still fairly naive.)

[somebee]

Thank you for taking the time to read through the code and trying to understand. What you did is turn caching off completely. I'm actually very suprised that Imba could still be faster in that case, I would have predicted it to be much slower than React and Mithril, as it doesn't reuse DOM nodes at all.

It is true that creating a million todos(!) in one second would take up lots of memory in Imba, unless you manually prune the cache (for now). I will answer this in more detail, but before that, I suggest you change the line to:

res.push((this['_' + i] = this['_' + i] || t$('todo')).setObject(todo).end());

With this change you will only ever cache as many nodes as there are visible tasks, yet not remove caching all together. This is not how you would usually do it in Imba though, the 'correct' way would be the way it is done in the benchmark.

PS! Sorry if I came of as harsh in my response on hackernews. Again, I'm really glad you took the time to really dig into the code :) And I'd love to get to explain the whole benchmark to you in more detail. You are the first to (correctly) observe that Imba does not automatically dereference previously cached nodes. This would be done with pruning (manually for now). The reasons are explained quite well by judofyr, I'll quote him for context:

Imba only uses === for diffing. If you want Imba to re-use a DOM node, you must use the same tag object. Imba provides syntax for caching nodes as properties on the current object.
For instance, in order to re-use DOM nodes in a for loop you do this:

for event in @events
  <event@{event.id} name=event.name>

The loop body compiles down to:

  (this['_' + event.id()] = this['_' + event.id()] || t$('event')).setName(event.name()).end()

The next time you render the view, Imba will find the tag objects that have been reordered (using ===) and move them to the right place.
One thing to be aware of is that Imba doesn't automatically remove the cache for you, because we've found that it's tricky to determine when you actually want to purge it. For instance:

if mouseOver
  <something>
else
  <something-else>

Just because mouseOver becomes true for one frame doesn't mean you want the <something-else>-tag to be purged from the cached and removed. Keeping it around is nice for performance and convenience (e.g. state, jQuery plugins).
In practice it turns out you want to purge whole pages/sections at the same time, which is way friendlier for the garbage collector as well.

[somebee]

If you had an app where you expected to have millions of todos I agree that you would not want to cache all nodes if they have been shown at least once. In those cases you would be much better off by doing something like this. But again, the benchmark is mostly trying to measure the time it takes to render the whole app when nothing has changed (in which case the this['_' + i] caching would be just as fast (and not create any additional tags). It just looked a bit boring when the benchmark appears to do nothing at all.

[somebee]

I have now actually commited this change to the benchmark. You are right that it might actually be better to not keep cached nodes for individual tasks hanging around. This is even simpler. Do you still think this is sneaky? If not, I'm happy to tell you that the performance actually increases on all benchmarks but 'everything', where it is the same. So 50x faster than React on my mbp here.

[somebee]

@curveship Do you have any other comments regarding this? Otherwise I guess I should close the issue, but I do want to make sure we're seeing eye to eye :)

[jrschumacher]

@somebee do you plan to address the points made by @curveship on Aug 22nd?

I'm interested in your response, specifically to "being fair to React"

if you're going to use Imba's performance strategies (caching and minimum update work), then you should be fair and use React's as well (shouldComponentUpdate). I don't follow the argument that you had to slow down React to be fair to Mithril (?), especially when Mithril has its analogue, { subtree: 'retain' }.

[somebee]

Hey. I haven't taken the time to respond to @curveship yet, but thanks for reminding me @jrschumacher.

I think most of the discussion here revolves around the fact that we still have crazy bad documentation and no real good description of our caching, why it is so fast, and why it matters for real world applications. The docs/site will be improved a lot in a week or two :) "Unchanged render" was originally the only benchmark, and still the only one I find enlightening, but since nothing happens visually while it runs, I decided to add a few that were a bit more 'visual'. Looking back, these should probably never have been added at all. I see that I still have work to do on explaining why this is the case.

I'll start with a comment I made over at the react discussion forums.

First things first, I had (amateurishly) absolutely no idea how fast Imba would be compared to regular virtual dom implementations. The way we do rendering in Imba was made long before we ever started thinking about benchmarking anything, and the TodoMVC app was made as a reference implementation, and then benchmarked.

The optimizations are extremely simple. but very difficult to do in React, since they require caching to happen inline in the code where you generate the components (see code). Since React is not intended to require JSX, it is not feasible to ask js-developers to manually cache all nodes etc. The approach is very different from React.

I am absolutely sure that the React way is better in many ways. It is easier to mess things up in Imba if you don't fully understand how the caching works. Since Imba does not do any 'intelligent' virtual dom diffing it would vastly decrease performance if you start mixing uncached nodes into these views. Imba really lacks documentation now, but we are working on it. I am confident that the performance difference is in fact real, and not only in special benchmarks, but in real world applications. I don't expect to convince any of you today, but I'm hoping to improve the documentation and get more examples out there to show how all this works. If you have an application which would create thousands of new views all the time, you will manually need to prune the cached nodes from time to time. We are working on automated ways to do this, but in all our projects we have developed using Imba over the last few years, we have never had a real world need for this.

Now for the main concerns raised by @curveship

Only hold onto the current live todo views

You have to admit that holding onto the rendered todo views when the original todo has been removed from your model (which usually means "gone from the universe") seems like it's tuned to the particular behavior of your benchmark, where such a todo suddenly reappears. This is a more conservative strategy than what you've got right now (hold onto max number of todos), but I think it makes a greater case for Imba's speed. I'm much more impressed by someone saying "we're 8x faster with a conservative implementation" than "we can tune to the benchmark and get 60x."

This is a valid point, and I did address the concern, which was the fact that the 'Everything' benchmark at one point removes a task and forces a rerender before appending it again. This was unintentional (and has been corrected). The change does not really affect the results. We could add a new task instead of reusing the previous one - but it was meant to simulate moving a task.

There should be a benchmark for rendering genuinely new todos.

DOM creation is an important part of a framework and should be tested. (Also, you're really fast here, though not 60x, so you might as well show that.)

You are more than welcome to add a benchmark that creates tons of tasks, but IMHO it would tell us very little of value. I've stated multiple times that I think all these frameworks are plenty fast if you only render on changes, and make sure to only render/resolve the changed parts. A benchmark for inserting tasks would to a much larger degree test the performance of DOM creation (in the browser). Imba is still much faster here, but how interesting is it to see how long it takes to insert 10 million tasks? Let's say you are organising your todos (in the real world) for 10 minutes, using a task manager. During these 36000 frames (assuming 60FPS) - how many new tasks would you add during this time? How many per frame? This is the type of micro benchmark that says very little about performance in real apps.

The 'Unchanged render' benchmark says a whole lot more. What it shows is that rerendering the whole view is now so inexpensive that it is possible to render on every single frame, without any need for listeners, bindings, dependency tracking etc. Keeping the view 'in sync' is no longer an issue. It is like rendering templates on the server - where you always know the whole state/story on render.

shouldComponentUpdate

if you're going to use Imba's performance strategies (caching and minimum update work), then you should be fair and use React's as well (shouldComponentUpdate). I don't follow the argument that you had to slow down React to be fair to Mithril (?), especially when Mithril has its analogue, { subtree: 'retain' }.

React and Imba can both optimize using shouldComponenUpdate (in Imba you would add the same logic inside node#commit). Utilising shouldComponentUpdate (in React and Imba) requires additional logic by the developer, not the framework. This is not an optimisation at the framework level - but an endpoint where the developer can write their own custom optimisations. Adding your own manual checks all around your app to check for statechanges is inelegant, cumbersome, and bug prone. In my opinion it defeats much of the purpose of a virtual dom, if you are expected to do your own diffing before sending it through. Also, the relative performance difference is about the same when turning it on, and since the original Mithril TodoMVC does not use subtree: retain, I have not taken the time to implement it there.

In addition, the way shouldComponentUpdate was implemented in the TodoMVC example (see here) it does not rerender unless the whole task object has been replaced. So in the original react example you would need to create a new task-object whenever you wanted to change anything on a task (title, completed-state etc). The lowlevel actions of this benchmark changes properties like task.title directly.

full update

I'm not sure I follow the value in a "full update" benchmark. We usually need two things: render the initial full view, and update as necessary based on changes. What's the need for testing full update speed?

Yes, but how do we know what to update, based on what changes? Are you showing the time elapsed since a task was created somewhere, do you have complex conditions that control whether a certain view is displayed or not? How do you register when the title of a task has changed? How do you know if this task is currently represented in the view?

Frameworks solves this in many different ways, and most of them require the use of listeners, bindings, and all sorts of tracking. This is in fact one of the main advantages of React - that the 'correct' way to sync the view is to call render on the top-level node, and let React take care of only rendering (as in touching the DOM) where there are actual changes. This makes development much easier, since you can practically call render whenever you know that something has changed.

When your app grows you still need to add shouldComponentUpdate guards etc to keep from resyncing everything, because the virtual dom is not blazingly fast even when it does not touch the dom. My claim is that Imba is so much faster at this, that you don't need any logic to keep parts of the app from rendering, nor do you need to track when things change at all. You can render every frame. Think about how you would write a server side template in a synchronous language/framework. It is very easy to make sure the whole view is in sync.

[jrschumacher]

@somebee thanks for linking to that response as well as your other points.

Side note: have you seen vuejs' performance comparison update http://vuejs.org/perf/. Seems like they had similar issues

[somebee]

@jrschumacher Thank you for linking to that :) To clarify for others who might read this.

Due to internal implementation differences, frameworks that uses async rendering (e.g. Vue, Om, Mercury) gains the advantage by skipping part of the calculations that happened in the same event loop. The real world user experience doesn’t demonstrate such dramatic difference.

This was one of my main concerns with the mentioned benchmark as well. In our benchmark everything happens synchronously, and all implementations are forced to do the same amount of synchronous updates. This can be seen by the counter in the title (Todos #number#) which is incremented on every full render. As we state in our todomvc-render-benchmark readme.md:

There has been a TodoMVC benchmark floating around earlier. It mainly tested the performance of your browser, by creating fake events and navigating the dom to insert, complete, and remove todos. Some frameworks such as elm and mithril outperformed React by a lot. The (only) reason for this was that they used asynchronous rendering, while React rendered / synchronized the whole view every time a todo was inserted, completed, and removed. Making rendering in React asynchronous would take 2 lines of code, and bring its performance up to the others.

[unabst]

Just curious @somebee what your take is on React's design decision to not make rendering asynchronous and faster? Most frameworks tout their superiority to React in terms of speed, yet, here the claim is React is purposefully keeping things slower, but to someone like me, I can't wrap my head around why. I know React is backed by the geniuses at facebook, so I've been wondering about this ever since I've been shopping around for a framework to use for my next project...

[somebee]

@unabst Thankfully, React has not made such a design decision, only the React implementation of TodoMVC. So if you use React, it is up to you whether you want to do rendering asynchronously (usually with requestAnimationFrame) or force synchronous rendering whenever something has changed. The same goes for Imba. In my own projects I usually render the whole application on every frame, but you could render as a reaction to certain events etc.

[unabst]

Ah, thank you. It has to do with the benchmark then, which makes much more sense. I am just curious, is there anything that would stop facebook from using Imba for the front end? Is there something about React that makes it worth the weight and complexity in certain scenarios (like say, for serving facebook content)? I guess I should really be reading the React docs, but they seem bias towards React :)

[lukeed]

Is there something about React that makes it worth the weight and complexity in certain scenarios

No.