goatslacker / alt

Isomorphic flux implementation
http://alt.js.org/
3.45k stars 322 forks source link

Emit store change is really slow #202

Closed mcwhittemore closed 9 years ago

mcwhittemore commented 9 years ago

I am currently testing out Flummox and Alt for a project and in my perf tests Alt is really great until I envoke store.listen(function(){}). Before I dive in and try to find a way to fix this, I was hoping someone could review my perfs?

With store.listen: http://jsperf.com/flux-capacitor

Without store.listen: http://jsperf.com/flux-capacitor/4

goatslacker commented 9 years ago

Good question I'm not really sure. The store emitting aspect is pretty dumb. Both flummox and alt use eventemitter3 so my guess is this is something else. Benchmarks are weird.

I'll dig in and try and check it out.

mcwhittemore commented 9 years ago

Thanks! I'll try to dive through it a bit tomorrow too and report back if I find something helpful.

goatslacker commented 9 years ago

Thanks for this issue btw, I've started to take a deep look at performance and have found some odd things in both alt and flummox.

goatslacker commented 9 years ago

So I found the issue with the jsperf benchmark.

Spoiler alert: It's not the store listener it's what you do inside of it.

First of all, both frameworks perform poorly in these tests. There's a lot going on!

So, when you're listening to the store, with alt, the first and only argument the store listener receives is the current state.

Your code:

window.altjs.store.listen(function(store){
  var items = window.altjs.store.getState().items;
});

What it should be:

window.altjs.store.listen(function(state){
  var items = state.items;
});

So it performs poorly because getState does a shallow-copy of the state before it's passed over to you so you don't accidentally mutate it. Whereas in the flummox example you get the direct copy of the state so you're able to do un-flux like things like:

 window.flummox.store.state.items.push('trololol')

I created a new example which has the proper way:

http://jsperf.com/flux-capacitor/6

Alt is faster but by a negligible amount. I wouldn't exactly call one faster than the other.

This issue did however get me thinking about perf. Here are some things I learned:

mcwhittemore commented 9 years ago

Nice catch. My co-worker questioned getState on friday, but the idea got dropped for some reason or another.

I don't think the perf of creating actions or stores really maters as they are done once for a rather limited number of entities. The dispatch loop on the other hand is run over and over again.