search

ilesinge / scuttloid

Semantic Scuttle android client.
GNU General Public License v3.0
14 stars 3 forks source link

Reduce Data Transfer / Faster Loading #8

Open pascalfree opened 10 years ago

pascalfree commented 10 years ago

Hi.

In its current state, the app downloads the complete list of bookmarks if its started freshly (not from RAM). Correct me if I'm wrong. To load my entire collection of bookmarks it currently takes about 4 seconds (in Wifi), which is not bad, but could be better and more efficient.

I've been thinking about a way to improve this. I'm posting my thoughts here before I try to implement it, because you may have some better ideas. After each single step the app should still work fine.

  1. Add a Class BookmarkLoader which acts as a proxy between the BookmarkListActivity and the ScuttleAPI. The BookmarkListActivity should only call methods from the BookmarkLoader which then calls the ScuttleAPI (for now).
  2. Add a Class, that can save and load the bookmarks and everything related to it from a local storage (preferably, one that doesn't need special permissions).
  3. Let the BookmarkLoader save the Bookmarks to the local storage, after downloading them. Also save the current timestamp somewhere. When loading the bookmarks, the BookmarkLoader, should first check, if there is local data. If there is, it should use the posts_update.php API function, to check if the bookmarks have changed on the server, since the last sync. If so, re-download all bookmarks
  4. This step will need the all?hashes API function, which is supported by the delicious API but not yet by the semantic Scuttle API. When implemented in the API, the BookmarkLoader should load and save the hashes locally. When the bookmarks have been changed server side (posts_update.php) the BookmarkLoader should load all hashes and compare them to the local database. Then it should get all the bookmarks that have changed (have a different meta-hash).

Adding and editing will not affect the local storage directly to avoid syncing issues.

I hope my description is understandable. :-) I will start with the implementation, when I find the time (and after finishing the other feature(s)). Any suggestions are welcome.

ilesinge commented 10 years ago

Yes, I had planned to implement something like this in Scuttloid. It would make it much more usable. And you described it very well so I say : go for it !

I only have a small question regarding "Adding and editing will not affect the local storage directly to avoid syncing issues." : what are you suggesting exactly ? That, for example when you create a bookmark, it would only call the remote API ? But then a fetch of the newest bookmarks should be forced so that the new bookmark appears in the local storage / in the list. And the same thing for editing.

pascalfree commented 10 years ago

Yes, that's what I suggest. Also I wanted to point out, that the local storage should NOT (yet) be used, to store changes, if the server is currently unreachable.

I was thinking about updating the local "last synced timestamp" after adding a bookmark, but that would be problematic in the following scenario:

  1. Something is modified on the server
  2. Something is added via scuttloid (which did not sync the last modification yet)

It's not a critical problem I think. One solution would be as you described. Another option would be:

  1. Show the new entry in the list. Also maybe save it to the local storage, but make it identifiable as "not synced from server" (e.g. leave the meta hash blank, which is unknown anyway). Also do not update the "last sync timestamp"
  2. on the next sync, replace it with the downloaded version.

In short: The server always wins :-)

So, I will work on this.

ilesinge commented 10 years ago

Depends on: https://github.com/cweiske/SemanticScuttle/pull/8

pascalfree commented 10 years ago

Hi. I've started working on a fall-back for old semantic scuttle APIs. It currently does the following:

Further, I'm planing to implement the following:

The changes are on the local-storage branch but not on the testing branch.

pascalfree commented 10 years ago

Quick update: The bookmarks are now refreshed (from server) when the server settings are changed. And I merged local-storage into testing.

pascalfree commented 10 years ago

I found the official implementation for the pull-to-refresh feature. Which was surprisingly difficult because it's called SwipeRefreshLayout. video of example. It is pretty simple to implement. The colors can be adjusted. I used light orange and light green in this first implement. The animation (colors moving around) will start whenever the bookmarks are refreshed from the server.

I put it on the local-storage branch but not in testing.

pascalfree commented 10 years ago

The Deletion Detection Workaround I implemented a workaround that works like this:

  1. Check the last update time using posts/update.
  2. If changes are detected, refresh the bookmarks (redownload) 3.a) for APIs without the bug: That's it. Stop here. 3.b) for APIs with the bug: Get posts/dates and create a HashMap (date->count)
  3. Compare the HashMap to the one created from the local Data in the Database
  4. If they don't match, bookmarks have been deleted: refresh bookmarks.

Note1: After implementing this I realized, that it would be easier to just compare the total number of bookmarks. However there is no API function to directly get the total number of bookmarks (afaik), so posts/dates is still the most efficient way to go.

Note2: A user may delete a bookmark of today and create a new bookmark. This will result in an equal HashMap, but it is not a problem, because Step 1 and 2 will detect the creation of the new bookmark.

Note3: The Database has been updated (The date of creation is now stored with the bookmarks). After installing this update it is not possible to go back without uninstalling the app first.

I think, with this workaround, the local storage becomes useful/usable even with the current API. Here are some more Improvements/Bugs related to this issue:

The workaround is on the local-storage branch

pascalfree commented 10 years ago

Incremental Sync The newer APIs (current delicious and future semantic Scuttle) are designed for incremental sync like this:

  1. Check for updates using posts/update.
  2. Get bookmark hash and meta, using posts/all?hashes, which indicate which bookmarks have changed. This download is about 1/3 of the size of posts/all. Also there is no limit of bookmarks for this function in delicious.
  3. Missing hashes indicate, that the bookmark has been deleted. Delete them locally.
  4. Different meta for the same hash indicate, that the bookmark has been changed. Download these (all at once) using posts/get. Replace/Add the Bookmarks locally.

I implemented this in fd8827c70718f17d0128bc14d56fe47613245117. It checks whether the API support incremental sync. If not, it falls back to downloading everything using posts/all.

About the Deletion Detection Workaround I realized, that the current version of semantic scuttle also has a Change Detection Bug, which has been fixed here https://github.com/cweiske/SemanticScuttle/pull/7. It will be released with the next version. I can not think of a workaround for this, so I will change back the behaviour for the current semantic scuttle API.

The next steps are still the same as in the last comment.

pascalfree commented 10 years ago

It's time for some updates :-) I fixed two of the problems mentioned two comments earlier. namely:

I also reverted the deletionDetectionBug Workaround, though some code related to it still exists (marked with TODO: deletionDetectionBug). I don't want to remove it until the next version of semanticScuttle is out.

So now the local-storage branch should be usable. But I'd like to test it more before merging it to testing/master. A Quick summary of the changes in this branch:

There is one thing left, that I would like to implement here:

And I found a bug in the current semanticScuttle API:

Delicious keeps the date. The Fix in scuttloid would be to send the original date of the bookmark when editing it. However the modification date will be set to the same date. Delicious will ignore that completely. I will also fix this for the next version of the semanticScuttle API.

The changes are on the local-storage branch but not on testing yet.