Paginate profile feed #750
Conversation
| } | ||
| Button(Localized.reportNote.string, role: .destructive) { | ||
| showingReportMenu = true | ||
| if !note.isStub { |
There was a problem hiding this comment.
This is a little feature that allows us to copy the ID of notes even if they are only stubs, which is helpful for debugging.
|
Just testing it out with xcode on my phone... it crashed once with nothing interesting in the logs and then when i went from notifications to view a profile i got this: 1e7764cb418a8bc3da01f3d124d2bbf6c13ae9252b01e6359b639af176d40ddf loading view data |
|
Thanks for testing @rabble. I haven't been able to reproduce this crash but I noticed while trying that deleting a note on your profile doesn't remove the note right away. I'll get that fixed and continue looking for the crash. |
| @@ -151,15 +151,16 @@ enum AuthorError: Error { | |||
| return fetchRequest | |||
| } | |||
|
|
|||
| @nonobjc func allPostsRequest() -> NSFetchRequest<Event> { | |||
| @nonobjc func allPostsRequest(since: Date = .now) -> NSFetchRequest<Event> { | |||
There was a problem hiding this comment.
since made my think that it will fetch all events posted after that date, but it actually does the reverse thing. Can you rename this?
| @@ -805,6 +806,77 @@ public class Event: NosManagedObject { | |||
| return nil | |||
| } | |||
|
|
|||
| // MARK: - Preloading and Caching | |||
| // Probably should refactor this stuff into a view model | |||
There was a problem hiding this comment.
We should refactor this, its mixing the model and the view. Can we do it now that we are working on this? Otherwise the code will quickly grow in complexity.
There was a problem hiding this comment.
I want to refactor this too but there are a few reasons I didn't want to do it in this PR:
- It will make the diff huge because every view that holds a reference to a
Eventwill need to change - I'm not sure how to make this work with
@FetchRequestwhich returns NSManagedObjects and notifies us when they change. If we used a view model we would need to wrap @FetchRequest or something to instantiate view models when new objects are inserted? Same with NSFetchedResultsController. We could make a separate object that manages this cache of data but then we have to figure out cache invalidation.
I opened #764 to come back and figure this out later.
Nos/Models/RelaySubscription.swift
Outdated
|
|
||
| /// A handle that holds references to one or more `RelaySubscription`s and provides the ability to cancel these | ||
| /// subscriptions. Will auto-cancel them when it is deallocated. Modeled after Combine's `Cancellable`. | ||
| class SubscriptionCancellable { |
There was a problem hiding this comment.
Can you move this class to its own file?
This is the first part of #223. It adds pagination to our relay communication on the profile feed. Here's are the functional differences for the user:
I ended up using a UICollectionView to do the pagination because it solves the problem of determining when a cell scrolls into view (which is hacky/inefficient in SwiftUI) and it supports the
UICollectionViewDataSourcePrefetchingprotocol which is great for loading cells before they scroll into view. LazyVStack is supposed to load cells before they scroll into view but it seems to do it just milliseconds before which isn't enough time to load expensive network resources. You will find the collection view code inPagedNoteListViewwhich relies onPagedNoteDataSource.To do the pagination I had to do some refactoring to the
RelayService. Instead of mapping oneRelaySubscriptionto a bunch of different Nostr subscriptions on different websockets I refactored so that oneRelaySubscriptionrepresents one Nostr subscription for one websocket/relay. I also added aSubscriptionCancellablethat bundles these subscriptions into a single object that auto-unsubscribes from them when it is deallocated. This inflated the size of the diff as every place we open a subscription changed slightly. This refactor allows me to keep track of the oldest event received for a given subscription per-relay, which means when it comes time to load the next page of events we actually make a slightly different request to each relay to load events older than the oldest one we have received. As far as I can tell this is the only reasonable way to page in Nostr, and this is how other apps are doing it, but it can sometimes result in fetching more events than we need. ThePagedRelaySubscriptioncontains most of this logic.I also started logging the length of the
RelayService.parseQueue(filter logs for "parse queue") which was really eye opening as when the app first opens we add tens of thousands of events to the parse queue and it can take ~30 seconds to clear them all out. Switching tabs can cause tens of thousands more to be added to the queue and extend the time further. This causes things like reposts, mentions, and author names to load slowly. I knew it was bad but not this bad. Pagination will reduce the number of events in the queue by at least a factor of 10.