Skip to content
This repository has been archived by the owner on Dec 18, 2019. It is now read-only.

[POC/WIP] Add Immutablejs #19

Closed
wants to merge 1 commit into from
Closed

Conversation

jvanbruegge
Copy link
Contributor

This should increase performance a lot, haven't tested it yet.
I just want to get a bit of feedback :)

@jvanbruegge jvanbruegge mentioned this pull request Apr 9, 2017
@Frikki
Copy link

Frikki commented Apr 10, 2017

This may internally increase performance, but converting immutable.js to ordinary JS or JSON is very expensive. I am yet to see an application that doesn’t need to persist its state, which means to have to make this conversion.

Operating on small data objects, which is often the case in apps, the difference between Ramda’s assoc and immutable.js is minimal. Immutable.js doesn’t have the ability to reuse the rest of the data. https://esnextb.in/?gist=e51d7eb1f9d037f01b0c1c650e49c9dc

@jvanbruegge
Copy link
Contributor Author

This would have to be a fork. You would not convert, you would expose the immutable API

@Frikki
Copy link

Frikki commented Apr 10, 2017

As an end-user, in an actual app, I would still have to convert from immutable.js to some data which can be persisted in the app’s infrastructure. Thus, making it quite expensive.

@jvanbruegge
Copy link
Contributor Author

What do you mean by persisted? Onionify is used to persist state. If you want to keep it after page reload, you will have a lot more explensive actions than converting some data (HTTP RTT)

@Frikki
Copy link

Frikki commented Apr 18, 2017

Persistence as in storing data in external storage, usually a DBMS.

@jvanbruegge
Copy link
Contributor Author

Many projects use Redux with Immutable.js and this usually works fine.
As said, the cost of using persistent storage is usually much higher than any conversion cost.
In addition you can just throttle the state$ to reduce conversion costs.

@Frikki
Copy link

Frikki commented Apr 19, 2017

the cost of using persistent storage is usually much higher than any conversion cost

So what? Now you have both the costs.

In addition you can just throttle the state$ to reduce conversion costs.

I can also avoid immutable.js ;)

@jvanbruegge
Copy link
Contributor Author

it depends on use case, really.
For stuff that stores frequently to a DB but does not have a lot of state changes per second, plain JS is better.
But for stuff like my real-time graph viz app using immutable data structures is a huge performance win

@Frikki
Copy link

Frikki commented Apr 19, 2017

As a closing note from me, we have a library that uses immutable.js, but now that we need to use it in an application that doesn’t use immutable.js, our adapter converts the data structure. When we inspect the application, we see that immutable.js is responsible for 67.5% of the memory consumption, in particular places where we see a slowdown.

To see how immutable.js affects memory consumption: https://jsfiddle.net/sn70x2p6/ (yes, milage may vary, which is why I’m very concerned about libraries making use of immutable.js)

@staltz
Copy link
Owner

staltz commented Apr 21, 2017

@jvanbruegge Thanks!
Since this PR replaces objects with Immutable.js, we need to find a way to allow choice. I think it's okay to allow Immutable.js as long it's not the only option. It's specially hard to have it as the only option before Immutable.js is properly rewritten for TypeScript (it may happen sometime in the future). So for now let's see what's the best way to provide this opt-in. :)

@jvanbruegge
Copy link
Contributor Author

Yes, I thought about a fork, but thats not the best solution

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants