eleybourn / Book-Catalogue

A book cataloging tool for Android phones.
https://github.com/eleybourn/Book-Catalogue/wiki
GNU General Public License v3.0
385 stars 186 forks source link

Future(current) bug in main view #286

Closed Grunthos closed 12 years ago

Grunthos commented 12 years ago

Experimenting with large collections of books has revealed a limit in SQLite that we are likely to experience.

For each group in an ExpandableListView that is expanded, we return a cursor.

If someone clicks 'Expand All', we return 1 cursor for each group.

If that person has somewhere in excess of 800 (not sure exact number) of groups, SQLite will run out of memory/file descriptors/something.

It seems that ExpandableListView keeps thos cursors open for the entire time.

Couple this with the 'fact' (garnered from google) that the memory requirements of an SQLite cursor are not small, and we probably have a problem in the future, or anyone with a large collection now.

[As an aside, we also have a bug where these cursors are not being closed when the BookCatalogue activity is 'stopped'...only when it is destroyed, resulting in excess cursors being kept in memory until a GC]

As someone with about 337 authors, I always scream when I accidentally click 'Expand All' (I can go make a cup of coffee while I wait for it to complete), I have an interest in this area as well.

I am not entirely sure what the best solution is. Some suggestions come to mind:

  1. In terms of returning cursors for sub-lists, my suggestion is that we should maybe return array cursors after extracting the data from the cursor ourselves. This may even be memory efficient. Just an array of Parcelables or Bundles might suffice. The onExpandGroup could refresh it (somehow?). Even if the sublist contained 1000 books, this is going to be considerably less than 1MB since it olny needs to contain some IDs and the text required for the related view.
  2. The 'expandAll' option shuld probably be removed, or come with a warning. We may even want to auto-collapse groups when another is expanded.

I'd be very interested in any other ideas!

Grunthos commented 12 years ago

Simply copying the entire cursor contents to a MatrixCursor and using that results in:

(a) Almost identical run time (b) 33% reduction in total memory used by loading a completely expanded list and (c) A reduction in active SQLite cursors from 337 to 1.

This seems like a win to me!

Grunthos commented 12 years ago

Fixed in V4.0