-
Notifications
You must be signed in to change notification settings - Fork 296
WIP Local relay model #3204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
danieldaquino
wants to merge
91
commits into
master
Choose a base branch
from
local-relay-model
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
WIP Local relay model #3204
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
looks like feeds aren't updating :( |
a3c724e
to
6b4287e
Compare
9ff9159
to
42d9d6d
Compare
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Changelog-Changed: Switched to the local relay model Changelog-Added: Notes now load offline Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
…ed behaviour Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
The widespread usage of the SubscriptionManager caused new crashes to occur when swapping apps. This was caused due to an access to Ndb memory after Ndb has been closed from the app background signal. The issue was fixed with improved task management logic and ensuring all subscription tasks are finished before closing Ndb. Signed-off-by: Daniel D’Aquino <[email protected]>
This commit fixes a crash that caused the app to crash when getting all the follows from a profile. This issue was caused by a use-after-free memory error on inherited transactions after the original transaction is deinitialized. The issue was fixed by introducing a reference count on all transactions and only deallocating the C transaction when the ref count goes to zero. Signed-off-by: Daniel D’Aquino <[email protected]>
Previously, HomeModel could listen to all subscriptions throughout the app, and it would handle reaction and repost counting. Once moved to the local relay model, HomeModel no longer had access to all subscriptions, causing those counts to disappear. The issue was fixed by doing the counting from ThreadModel itself, which better isolates concerns throughout the app. Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
This commit implements nostr network subscriptions that survive between sessions, as well as improved handling of RelayPool opening/closing with respect to the app lifecycle. This prevents stale data after users swap out and back into Damus. Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Changelog-Changed: Added UX hint to make it easier to load new notes Signed-off-by: Daniel D’Aquino <[email protected]>
Changelog-Changed: Improved loading UX in the home timeline Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Changelog-Changed: Increased transaction list limit to 50 transactions Signed-off-by: Daniel D’Aquino <[email protected]>
This commit improves the loading speed for the home timeline (and likely other areas of the app) by employing various techniques and changes: - Network EOSE timeout reduced from 10 seconds down to 5 seconds - Network EOSE does not wait on relays with broken connections - Offload HomeModel handler event processing to separate tasks to avoid a large backlog - Give SubscriptionManager streamers more fine-grained EOSE signals for local optimization - Only wait for Ndb EOSE on the home timeline for faster loading - Add logging with time elapsed measurements for easier identification of loading problems Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Updates are streamed from the network, removing the need for a refresh action Signed-off-by: Daniel D’Aquino <[email protected]>
33aa214
to
eda4212
Compare
This reduces the overall subscription usage throughout the app, thus reducing issues associated with too many subscriptions being used at once, and the resulting staleness. Signed-off-by: Daniel D’Aquino <[email protected]>
Through some local experimentation, it seems that network relays can support higher subscription limits. Increase internal limits to avoid hitting issues with subscriptions waiting on subscription pool to clear and appearing stale. Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
This should prevent background crashes caused by race conditions between usages of Ndb and the Ndb/app lifecycle operations. Signed-off-by: Daniel D’Aquino <[email protected]>
This should prevent RUNNINGBOARD 0xdead10cc crashes related to ProfileManager and app background states. Signed-off-by: Daniel D’Aquino <[email protected]>
This commit adds more safeguards to prevent RUNNINGBOARD 0xdead10cc crashes, by: 1. Using the `beginBackgroundTask(withName:expirationHandler:)` to request additional background execution time before completely suspending the app. See https://developer.apple.com/documentation/xcode/sigkill 2. Reorganizing app closing/cleanup tasks to be done in parallel when possible to decrease time needed to cleanup resources. Signed-off-by: Daniel D’Aquino <[email protected]>
Use Apple's unified logging system, and specify proper privacy levels for each piece of information. Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
- Resend subscription requests to relays when websocket connection is re-established - More safeguard checks on whether Ndb is opened before accessing its memory - Cancel queued unsubscribe requests on app backgrounding to avoid race conditions with subscribe requests when app enters the foreground - Call Ndb re-open when Damus is active (not only on active notify), as experimentally there have been instances where active notify code has not been run. The operation is idempotent, so there should be no risk of it being called twice. Signed-off-by: Daniel D’Aquino <[email protected]>
Previously, we combined the ndb and network stream within a "session subscription" stream, which was teared down and rebuilt every time the app went into the background and back to the foreground (This was done to prevent crashes related to access to Ndb memory when Ndb is closed). However, this caused complications and instability on the network stream, leading to timeline staleness. To address this, the pipeline was modified to merge the ndb and network streams further upstream, on the multi-session stage, allowing the session subscription streams to be completely split between Ndb and the network. For the ndb stream, we still tear it down and bring it up along the app foreground state, to prevent memory crashes. However, the network stream is kept intact between sessions, since RelayPool will now automatically handle resubscription on websocket reconnection. This prevents complexity and potential race conditions that could lead to timeline staleness. Signed-off-by: Daniel D’Aquino <[email protected]>
…it timeline staleness Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
This improves upon a temporary fix we had for the RelayPool race condition that would cause timeline staleness. The root cause was that during app launch, the HomeModel would subscribe to some filters, and the subscribe function would filter out any relays not yet connected to avoid unnecessary waiting for EOSEs from disconnected relays. However, that filtering would cause the subscribe request to not be queued up or sent back to the relays once connected, causing the relays to never receive those subscription requests and causing timeline staleness. This was fixed by separating the relay list used for the subcription request from the relay list used for waiting for network EOSEs. This allows other mechanisms to ensure the subscription will go through even when the app is initializing and relays are not yet fully connected. Fixes: 61eb833 Signed-off-by: Daniel D’Aquino <[email protected]>
This feature is not production-ready, and is not essential for the current scope of work, so descoping it and hiding it behind a feature flag until it is ready. Changelog-Removed: Removed "Load new content" button Signed-off-by: Daniel D’Aquino <[email protected]>
Closes: #3252 Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
This attempts to improve the performance of InnerTimelineView by performing event filtering computations on "EventHolder.insert" instead of on each view body re-render, to improve SwiftUI performance. Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
This is to reduce the amount of computation it takes to create the EventActionBar view Signed-off-by: Daniel D’Aquino <[email protected]>
This is a large refactor that aims to improve performance by offloading RelayPool computations into a separate actor outside the main thread. This should reduce congestion on the main thread and thus improve UI performance. Also, the internal subscription callback mechanism was changed to use AsyncStreams to prevent race conditions newly found in that area of the code. Changelog-Fixed: Added performance improvements to timeline scrolling Signed-off-by: Daniel D’Aquino <[email protected]>
Signed-off-by: Daniel D’Aquino <[email protected]>
Changelog-Changed: Optimized network bandwidth usage and improved timeline performance Signed-off-by: Daniel D’Aquino <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Warning
Still very experimental. It compiles and many features seem to be functional, but many things are still broken
Checklist
Closes:
orFixes:
tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patchesTest report
Please provide a test report for the changes in this PR. You can use the template below, but feel free to modify it as needed.
Device: [Please specify the device you used for testing]
iOS: [Please specify the iOS version you used for testing]
Damus: [Please specify the Damus version or commit hash you used for testing]
Setup: [Please provide a brief description of the setup you used for testing, if applicable]
Steps: [Please provide a list of steps you took to test the changes in this PR]
Results:
Other notes
[Please provide any other information that you think is relevant to this PR.]