scuttloid icon indicating copy to clipboard operation
scuttloid copied to clipboard
verified publisher icon ilesinge

Reduce Data Transfer / Faster Loading

Open pascalfree opened this issue 11 years ago • 9 comments

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.

pascalfree avatar Feb 03 '14 20:02 pascalfree

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.

ilesinge avatar Feb 09 '14 17:02 ilesinge

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.

pascalfree avatar Feb 10 '14 22:02 pascalfree

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

ilesinge avatar May 18 '14 08:05 ilesinge

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

  • When opening the app, the bookmarks are loaded from the local database.
  • When the refresh button is pressed, the bookmarks will be loaded from the server and the list will be updated once the sync is finished.
    • For now the progress icon is shown in the action bar, such that the list is not blocked while syncing
    • The posts/update check is skipped for older (i.e. the current) semantic scuttle API versions, due to the deletion bug. If delicious.com is used, the check will be done.

Further, I'm planing to implement the following:

  • Start a sync if the ~~local database is empty~~ settings are changed (otherwise the app may seem broken to new users)
  • Add an option in the settings to enable refresh on startup (when enabled for old API versions the app will behave as it currently does, i.e. download everything on startup)
    • An additional option to enable it on wifi only
  • ~~Replace the progress icon with a progress bar~~
  • ~~Implement pull-to-refresh (alternative)~~
  • Use the SwipeRefreshLayout
  • [note to self] Maybe it is possible to use the posts/dates API function to work around the deletion bug

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

pascalfree avatar Jun 15 '14 21:06 pascalfree

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

pascalfree avatar Jun 27 '14 12:06 pascalfree

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 avatar Jul 04 '14 11:07 pascalfree

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:

  • Add an option in the settings to enable refresh on startup (when enabled for old API versions the app will behave as it currently does, i.e. download everything on startup)
    • An additional option to enable it on wifi only
  • [done] Reload Bookmarks after updating the Database. Otherwise the Database is empty i.e. no bookmarks are shown
  • [done] Change/Add/Delete Bookmarks in the local database after changing them on the server via scuttloid (or force a refresh). Otherwise the user must refresh them manually, which is counter-intuitive. [done]
  • [obsolete] Simplify the HashMap comparison. Just compare the total bookmark count instead

The workaround is on the local-storage branch

pascalfree avatar Jul 30 '14 20:07 pascalfree

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 avatar Aug 03 '14 15:08 pascalfree

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

  • Change/Add/Delete Bookmarks in the local database after changing them on the server via scuttloid
  • Reload Bookmarks after upgrading the Database.

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:

  • A local SQLite Database is used to store the bookmarks offline
  • A BookmarkManager will keep the database up to date. It operates between the Activities and the Storage/API
  • Pull-to-Refresh (requires Extras/Android Support Library to compile!)
  • For the current version of SemanticScuttle:
    • On startup the bookmarks are loaded from the local database.
    • When the user refreshes manually, all the bookmarks will be redownloaded and saved offline.
  • For Delicious and the future SemanticScuttle API:
    • Also load local bookmarks on startup
    • On refresh, only the bookmarks that have been changed/created will be downloaded using at most 3 API calls: posts/update, posts/all?hashes, posts/get?hash=....

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

  • Add an option in the settings to enable refresh on startup (when enabled for old API versions the app will behave as it currently does, i.e. download everything on startup)
    • An additional option to enable it on wifi only (may require additional permissions)

And I found a bug in the current semanticScuttle API:

  • When a bookmark is edited, the date of creation will be updated.

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.

pascalfree avatar Oct 26 '14 17:10 pascalfree