ReviewOS

craft-native/craft

getContacts runs 2N+1 queries with a null projection, on the JavaBridge thread

#165
Closed chris opened this 9 days ago ·
9 days ago

CraftBridge.getContacts reads the contact list, then runs two more queries per contact:

val cursor = activity.contentResolver.query(
    ContactsContract.Contacts.CONTENT_URI,
    null, null, null, ContactsContract.Contacts.DISPLAY_NAME + " ASC")

cursor?.use {
    while (it.moveToNext()) {
        val id = it.getString(it.getColumnIndexOrThrow(ContactsContract.Contacts._ID))
        ...
        put("phoneNumbers", getContactPhones(id))   // one query
        put("emailAddresses", getContactEmails(id)) // one more
    }
}

getContactPhones and getContactEmails each open a cursor against Phone.CONTENT_URI / Email.CONTENT_URI filtered to that one contact. So a single craft.contacts.getAll() on a device with 2,000 contacts makes 4,001 queries, each a Binder round trip to the contacts provider.

Three things make it worse than the count suggests:

  • The projection is null on all three queries, so every column of Contacts — a wide, denormalized view — and every column of the data rows crosses the Binder boundary, when the code reads two columns and one respectively.
  • It runs on the JavaBridge thread, so nothing else the page calls through CraftAndroid is served until it finishes.
  • The promise has no timeout (it is hand-built, like the rest), so a page that calls this on a large address book has no way to give up.

The shape that would fix it

Three queries total, grouped in memory:

  1. Contacts with projection = arrayOf(_ID, DISPLAY_NAME), sorted as now;
  2. Phone.CONTENT_URI with projection = arrayOf(CONTACT_ID, NUMBER) and no selection;
  3. the same for Email.

Then bucket 2 and 3 by CONTACT_ID into a map and join. Same output, one round trip per table.

The behavioural difference worth naming: with 4,001 queries, an address book edited midway through produces a mixture; with three, a smaller and differently-shaped mixture. Neither is consistent — ContentResolver gives no snapshot across queries — so this is a change in which inconsistency you get, not a regression from a consistent state.

Zig ports it as-is

bridge_android_contacts.zig reproduces the 2N+1 loop deliberately, so the port stays auditable against the Kotlin line by line. It is the wrong algorithm in both languages now instead of one — which is the honest position while the question is open, and the module comment says so.

9 days ago

Implemented and pushed in 691fb93. Android contact loading now uses one projected contacts query plus one projected phone query and one projected email query, groups related values in memory, and preserves contact ordering/JSON shape. Added Kotlin generator and Zig bridge regression coverage. Exact-commit CI, SBOM/vulnerability scan, Binary Size, and Benchmark workflows all completed successfully.

Sign in to comment on this issue.