[17:58:46] dbrant: niedzielski-afk: i'm beginning to think that piggybacking off of ReadingListPageObserver was the wrong way to go for updating the feed and reading list UI on page save events. with the synchronization patch (rebased over this fix https://gerrit.wikimedia.org/r/#/c/348076/) it's causing an awful amount of flicker on the feed with all the db disk [17:58:46] events [18:00:07] it'd probably be better to write up a purpose-built event listener/dispatcher [18:00:24] ReadingListPageObserver just fires too frequently [18:00:46] perhaps that's right [18:11:38] hmmm [18:11:43] now i'm having second thoughts about that [18:11:52] immediate UI updates if needed is what we want [18:12:38] we just need a way of limiting them to when there's content that actually needs updating [18:14:50] which still requires listening for something finer-grained than "something changed in the reading list page database." so i guess my original thought stands. [18:15:11] *end monologue* [18:16:28] an observer-type receiver feels like the right thing to use. but is there a way within the observer to tell what actual change triggered it? [18:21:09] dbrant: i don't think so, not with a ContentObserver at least. the only way i can see to narrow things down is to register specific observers for each specific page/row we're interested in, for as long as we're interested in it [18:21:25] which would get the job done but feels awfully cumbersome [18:23:42] well... maybe i'll play with that idea a bit [18:24:28] anyway, syncing is looking pretty good with the new patch [18:25:54] ;) [18:29:47] mdholloway dbrant: there are some good observation and IIRC sync adapter examples in the google io app [18:30:27] https://github.com/google/iosched/blob/master/android/src/main/java/com/google/samples/apps/iosched/provider/ScheduleProvider.java [18:31:19] niedzielski: thanks, looking now [18:35:43] mdholloway dbrant: i tried to mimic their contract style for provider URIs and i think there's somewhere in the code they talk about a specific case of throttling in one of the observers [18:39:10] mdholloway dbrant: this wasn't what i was thinking of but it sounds promising: https://github.com/google/iosched/blob/master/android/src/main/java/com/google/samples/apps/iosched/util/ThrottledContentObserver.java [18:42:13] throttling seems like kind of a blunt solution, but interesting [18:53:55] mdholloway: i think another way is to create a new provider and new contract that will allow you to observe only the change you care about (as opposed to strict row URIs). it's been a while since i've touched that part of the code but i think we have different URIs for things like observing a disk status + a page image and such. they're hierarchical [18:56:49] mdholloway: i think it's like if you can construct the hierarchy to the thing you care about most first, that's best. for example, i think the uri foo/bar/baz/page/image would allow you to observe image changes but you'd also get page changes? and foo/bar/baz/image/page would allow you to observe page changes but you'd also get image changes? it was something like that [18:59:35] mdholloway: i think that when you care about multiple things, you either have to use the coarsest endpoint or observe multiple endpoints. (sorry if any of this is wrong but i hope it at least points in the right direction-- i'm trying to wrap up a separate patch and haven't touched these observers in a while.) [19:00:29] (niedzielski: sorry, i got pinged in -parsoid but i'm listening -- thanks for the info!) [19:01:41] 👍 [19:51:31] niedzielski: ok, so yes. ReadingListPageObserver is registered to observe changes on content://org.wikipedia.dev/readinglist/page/disk. It also fires (rather obnoxiously) on content://org.wikipedia.dev/readinglist/page but we have a check to filter these out. still, when a bunch of changes get synced, that's a lot of events. [19:51:34] niedzielski: listening on specific rows seems kind of absurd at first blush but it would also allow us to make targeted notifyItemChanged notifications instead of having the system recreate the entire view hierarchy on every change with notifyDataSetChanged [19:52:21] which we'd still have to do, just less often, if throttling [19:53:01] i believe there are also some examples of batching writes but i'm not sure how applicable they'll be [19:54:04] but yeah, targeted ui updates sounds good [19:54:49] that was what got me thinking seriously about per-row observers even though i failed to articulate it out loud :| [19:57:13] i think the write URI endpoints can also make a call as to whether to emit a notification that the row has changed. it would be nice to not emit notifications for updates that effectively write the same value but i think the sql + android sql api for to do this check might be tricky [19:57:32] iirc, that's why we do upserts [20:22:08] jdlrobson: i made a note to bring up tag issue at next retro