genkgo / ember-localforage-adapter

Offline usage for Ember Data, based on localstorage adapter, but now uses Mozilla's localforage as data source
Other
133 stars 26 forks source link

Discussion about implementation #43

Closed sebweaver closed 5 years ago

sebweaver commented 9 years ago

A I said here and there that I have some ideas to improve the whole implementation of ELA.

Purposes are:

  1. Make ELA even thinner than the version discussed in #42
  2. Improve its overall performance, specially on the memory footprint

Let me explain my POV.

I use ELA in an mobile application using Cordova. This application is designed to work offline most of the time, so ELA let me store my data locally without any concern of the underlying storage (thanks to LF). The problem is I have a lot of data with a lot of relationships.

Issue #42 already addresses a part of this issue by avoiding to preload unnecessary relationships. Which, in my case BTW, lead to load almost the whole database. However, there are still 3 remaining issues:

  1. The memory consumption
  2. The I/O bandwidth
  3. The lack of granularity

Memory consumption

The current implementation load all records in memory, optionally put them in cache, and copy them to the store during the loading process (without #42) or asynchronously (with #42). So in a worst case scenario, when application requires all the data, every single record of the database could exist at least twice in memory:

If the database is very big, this could be problematic depending on the available local resources.

I/O bandwidth

In the current implementation the whole database is stored in LF as a single value, under a single key. This way, every single record CRUD lead to read and/or write the whole database in one shot!

As for the previous issue, if the database is very big, this could be problematic depending on the available local resources.

Lack of granularity

Even if the database is light, the previous issue could lead to inconsistent issues if the something bad happens during the read/write process, or even worse, break the entire database.

This issue refers to the ACID principle, which is not respected here.

Proposed solution

When I used ELA to the first time I was really intrigued about the choice of implementation. I was so happy to find an adapter which solved my needs that I ended up to get used to it. I also said to myself they were certainly some pretty good reasons to implement it that way.

However, it led me to a unfortunate misinterpretation: to improve granularity, I decided to split my models into different namespaces. Big mistake, the current loading process of relationships can't see across namespaces and relationships were never satisfied. So I had to move my models under the same namespace. I concluded that namespaces were designed for other needs, may be when applications require different data models which have to be hermetic with each other.

Note: since the loading process is about to be removed by #42 and handled natively by the store, it might work now. But I did not test again and above all, the new implementation could make this issue obsolete.

Anyway, here is the thing:

LF isolation level Original purpose Current usage in ELA Proposed usage
name Name of the database Unused (fixed by config) Name of the database (i.e. namespace)
storageName Name of the key/value table Unused (fixed by config) Name of the model
key Key to get a value Namespace ID of the record
value Gold nugget Entire structure of the database (records grouped by model) Content of the record (attributes, relationships)

This table shows that we lose at least 2 precious levels of isolation with the current implementation. The proposed usage aim to stick to original purpose of LF structure more naturally.

Note: this implementation implies there will be as many LF instances as namespaces. The latter could be referenced in ELA thanks to a container (which could be a service too) in order to dispatch operations according to a given namespace.

Benefits

  1. Stick naturally to the structure of LF
    • Thinner and easier implementation
    • CRUD operations become atomic
    • Taste more ACID
  2. Closer to any database structure (where tables represent models and rows represent records)
    • Better understanding for developers
  3. Closer to any remote adapter:
    • Data are where they are and stay there, the store load or unload them asynchronously on demand
    • Could consequently inherit and specialize existing standard adapter (JSONAPIAdapter for example)

Issues solved?

  1. About memory footprint:
    • If cache is enabled, the benefit is almost irrelevant compared to current implementation because records continue to exist twice in memory (in the cache and in the store)
    • However if cache is disabled records only exist once in the store (see previous point _#_3)
  2. About I/O bandwidth
    • Like any other remote adapter, nothing more, nothing less
    • Atomic operations! (see previous point _#_1)
  3. About granularity
    • As much flexible as LF allows it by design
    • Since each value represent a single record, atomic CRUD operations reduce the risk of damaging the database in case I/O problem (see point _#_1)

Pitfalls

The new implementation can't be compatible with existing databases. Database structure is slightly different. Serialization could even be changed too (using JSONAPISerializer for better compatibility in the future). There are 3 solutions to address this pitfall:

  1. Provide a migration process to transform an existing database to its new structure
    • Nice but complex and risky
  2. Maintain alive 2 versions of ELA and switch between them by configuration (new mode vs legacy mode) of by differentiating imports
    • Always boring to have to maintain multiple versions of a project, but also the safer solution
  3. Completely fork the project
    • Close to solution 2, but each project can follow its own path
    • Could confuse users though...

What do you think about all of this? Was there any specific reason to choose this implementation in the first place? I may have missed something, if so, I'd really want to know what.

Note: as a non native english speaker, I hope my english was clear enough to explain this big task :-)

Regards,

Sébastien

sebweaver commented 9 years ago

Reading LF project's history, It seems that the support of multiple instances is pretty recent. It could be one of the reasons to the current implementation of ELA. The fork from the ember-localstorage-adapter is certainly another one, since the latter doesn't support multiple database/store per domain.

However, thanks to the current abstraction layer of LF it makes the proposed implementation now possible.

frederikbosch commented 9 years ago

@sebweaver Again, I am in favour of all of this. My suggestion is to change one thing at a time where possible. The tests we have, help a lot of in this case. Regarding pitfalls, I think migration should be no problem since we are dealing with pojo and arrays.

sebweaver commented 8 years ago

I plan to look into this new implementation before the end of the month.

frederikbosch commented 8 years ago

We might want to consider running localforage in a worker. It might increase performance. I see that support for workers within Ember is premature. There are a few projects/addons on this topic. But we might not need those addons when we only localforage itself is running in a worker. There should support for that. Myself I also have never used workers before, but I think it is worth checking it out.

sebweaver commented 8 years ago

Eventually I did not have enough time on December/January to look into this.

Your suggestion to run LF in a worker is a good idea and I'll keep that in mind!

I'll keep you posted whether and when I could reschedule all of this.

frederikbosch commented 8 years ago

@sebweaver No worries, totally understood. The library is in a fine shape anyway!

Ramblurr commented 8 years ago

I'm evaluating moving from ember-local-storage (not to be confused with ember-localstorage-adapter to localforage on a cordova backed mobile app.

The goal is to increase performance, especially on mobile safari. My app's database is quite large.

I'm also eyeing a sqlite driver for localforage as a possibility to increase performance.

Running across this issue however has given me pause. The single-key based architecture was why I jumped ship from ember-localstorage-adapter to ember-local-storage. It is simply silly to be serializing/deserializing the entire database on every write operation.

If you're interested in an alternative implementation, checkout ember-local-storage. Each model is stored in its own key of the format modelName-ID (e.g., posts-1) and each model has an index key of the format index-modelName (e.g., index-posts). ember-local-storage also supports querying on relationships.

I didn't mean for this comment to become an advertisement for e-l-s. It clearly has its limitations (localStorage), but it has addressed some of the topics in this issue.

Frankly, I'd love to switch to LF, but based on the discussion in this post, it seems it would be a step backward.

frederikbosch commented 8 years ago

@Ramblurr Very welcome to post your findings. Does not feel like an advertisement at all for e-l-s. Actually I understand really well where your comment is coming from. The funny thing is that I made a fork of the ember-localstorage-adapter almost two years ago and changed it to a localforage adapter. But I did not change the methodology, only the storage mechanism was changed. And many people helped moving it to the next levels with ember-cli, es6 etc.

Now we should make the next step: change the methodology. But to be honest: I think I cannot find the time to make this step right now. The app that I am using LF for is working perfectly with this adapter. However, I still hope someone can push this library a big step forward. Then I am sure I will be able help on the minor bits too.

sebweaver commented 8 years ago

I wrote this issue when I worked on an application where the database was big enough to notice the performance drop. Unfortunately this application has never got beyond the POC stage. Since the single-key implementation was sufficient to make it work as intended, the new implementation progressively left my list of priorities...

If I had to finally release this application on production, or another one requiring the same architecture, there is no doubt that I would be back on track towards this new implementation.

In the meantime, new contributors for this issue are warmly welcomed! :smile:

fsmanuel commented 8 years ago

Hey! I'm the author of ember-local-storage and I'm interested in a collaboration or a implementation of localForage into ember-local-storage. I haven't worked with localForage yet but will dive into it as soon as possible.

frederikbosch commented 8 years ago

@fsmanuel At this moment, both @sebweaver and I, are not using the LF adapter. No one else has taken over development. You are free to help this package, or help your own package with the things that we have build.

fsmanuel commented 8 years ago

@frederikbosch I see! Thanks for the offer. I guess the tests are of great value. I'll get back to you as soon as I have a better understanding of LF.