update plan
This commit is contained in:
@@ -129,6 +129,12 @@ Action handlers can be registered on an element via the event handler `.on_actio
|
|||||||
|
|
||||||
When a view's state has changed in a way that may affect its rendering, it should call `cx.notify()`. This will cause the view to be rerendered. It will also cause any observe callbacks registered for the entity with `cx.observe` to be called.
|
When a view's state has changed in a way that may affect its rendering, it should call `cx.notify()`. This will cause the view to be rerendered. It will also cause any observe callbacks registered for the entity with `cx.observe` to be called.
|
||||||
|
|
||||||
|
GPUI also tracks the entities a window reads while rendering. When an entity notifies, every view that read that entity during the previous frame is rerendered automatically.
|
||||||
|
|
||||||
|
* A view that reads an entity in render (e.g. `state.read(cx).profile(...)`) does not need `cx.observe(&entity, ...)` to pick up its changes - the read itself is the dependency. Reserve `cx.observe` and `cx.subscribe` for when the view must run logic in response to a change (rebuild derived state, re-query, dedupe), not to trigger a re-render.
|
||||||
|
* Notify the narrowest entity holding the change. Notifying a global store rerenders every view that ever read it; notifying a per-panel entity rerenders only the panels reading it.
|
||||||
|
* This works with cached panels (`panel.cached(...)`): a cached subtree repaints when the view it belongs to is invalidated.
|
||||||
|
|
||||||
## Entity events
|
## Entity events
|
||||||
|
|
||||||
While updating an entity (`cx: Context<T>`), it can emit an event using `cx.emit(event)`. Entities register which events they can emit by declaring `impl EventEmitter<EventType> for EntityType {}`.
|
While updating an entity (`cx: Context<T>`), it can emit an event using `cx.emit(event)`. Entities register which events they can emit by declaring `impl EventEmitter<EventType> for EntityType {}`.
|
||||||
|
|||||||
@@ -0,0 +1,230 @@
|
|||||||
|
# Per-view profile directories
|
||||||
|
|
||||||
|
Status: proposal
|
||||||
|
Reference: coop `a365a8f` "bind user metadata to per-view directories"
|
||||||
|
|
||||||
|
## Background
|
||||||
|
|
||||||
|
`ProfileStore` (`crates/signed_state/src/profile.rs`) is a single global entity:
|
||||||
|
|
||||||
|
- Warm-loads the 500 most recent metadata events at startup (`WARM_LIMIT`).
|
||||||
|
- `get(pk)` returns a cached clone or a placeholder and queues a network fetch
|
||||||
|
(flume channel, 500 ms batching, chunks of 100, `sync_bootstrap_only`).
|
||||||
|
- Subscribes to `BackendEvent::ProfileUpdates` to re-query the local database.
|
||||||
|
- Every view reads through `ProfileStore::global(cx)`.
|
||||||
|
|
||||||
|
Problems:
|
||||||
|
|
||||||
|
- Invalidation is global: every `cx.notify()` on the store invalidates every
|
||||||
|
panel that read it, so any profile update repaints all profile-consuming
|
||||||
|
surfaces (including `panel.cached` ones), not just the panel whose data
|
||||||
|
changed.
|
||||||
|
- Startup work and memory scale with the local database, not with what is on
|
||||||
|
screen, and the global map never shrinks.
|
||||||
|
- Every screen shares one data set, so nothing is cancellable or reclaimable.
|
||||||
|
|
||||||
|
## Goals
|
||||||
|
|
||||||
|
- Profile data is owned per panel/screen. A directory dies with its panel,
|
||||||
|
cancelling outstanding work and dropping its cache.
|
||||||
|
- An update re-renders only the panel whose data changed: reads during render
|
||||||
|
are tracked by the window, and `panel.cached` repaints a panel as soon as
|
||||||
|
its directory notifies.
|
||||||
|
- Local database first: query the local cache, fetch from relays only for keys
|
||||||
|
the directory cannot resolve.
|
||||||
|
- One shared network pipeline (batching, dedupe) so N panels still produce one
|
||||||
|
relay sync per key.
|
||||||
|
|
||||||
|
Non-goals: changing `Profile` accessors, changing `Backend`, persistence.
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
Two pieces replace the fat global:
|
||||||
|
|
||||||
|
1. `ProfileStore` (global, thin): network fetch service + event broadcaster.
|
||||||
|
Holds no profiles and does no warm load.
|
||||||
|
- `fetch(keys)`: dedupe via an in-flight set, batch via the existing flume
|
||||||
|
channel + 500 ms timeout, chunks of 100 through `sync_bootstrap_only`.
|
||||||
|
- Emits `ProfileEvent::Updated(Vec<PublicKey>)` (a) after each synced batch
|
||||||
|
and (b) when `BackendEvent::ProfileUpdates` arrives from the pump.
|
||||||
|
2. `ProfileDirectory` (per panel/screen entity): the metadata one surface
|
||||||
|
renders.
|
||||||
|
- `track(keys, cx)`: queries the local database for missing keys, inserts
|
||||||
|
what it finds, queues the rest for a network fetch.
|
||||||
|
- `profile(key, cx)`: cached value or placeholder; a miss marks the key
|
||||||
|
pending and queues a fetch (same call shape as today's `get`).
|
||||||
|
- Subscribes to `ProfileEvent::Updated`, refreshes only keys it tracks or
|
||||||
|
has pending, notifies itself on change.
|
||||||
|
- Stores its own tasks and subscription; both drop with the panel.
|
||||||
|
|
||||||
|
```
|
||||||
|
panel view ──track(keys)──▶ ProfileDirectory ──DB query──▶ local database
|
||||||
|
▲ │ keys not found
|
||||||
|
│ cx.notify() ▼
|
||||||
|
└────refresh(authors)── ProfileEvent::Updated ──── ProfileStore (global)
|
||||||
|
▲ │ batch +
|
||||||
|
│ │ sync_bootstrap_only
|
||||||
|
BackendEvent::ProfileUpdates ▼
|
||||||
|
relay pump ─────────────▶ relays
|
||||||
|
```
|
||||||
|
|
||||||
|
Panels own `Entity<ProfileDirectory>` and read it directly in render; no
|
||||||
|
`cx.observe` is needed. gpui-pre records which entities a window reads while
|
||||||
|
drawing (`App::record_entities_accessed`) and, on `cx.notify`, invalidates
|
||||||
|
exactly the views that read them (`App::notify` → `Window::invalidate_view`).
|
||||||
|
This is the mechanism coop relies on — its `Directory` only calls
|
||||||
|
`cx.notify()` — and it composes with `panel.cached(...)`, which repaints a
|
||||||
|
panel when the panel view itself is invalidated. Scoping data per directory is
|
||||||
|
what scopes invalidation: notifying the old global store would invalidate
|
||||||
|
every panel that ever read it, while notifying one directory invalidates only
|
||||||
|
the panels reading it.
|
||||||
|
|
||||||
|
## API sketch
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// signed_state/src/profile.rs
|
||||||
|
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub enum ProfileEvent {
|
||||||
|
/// Metadata for these authors may have changed in the local database.
|
||||||
|
Updated(Vec<PublicKey>),
|
||||||
|
}
|
||||||
|
|
||||||
|
impl EventEmitter<ProfileEvent> for ProfileStore {}
|
||||||
|
|
||||||
|
/// Thin global: batches metadata requests from directories and syncs them
|
||||||
|
/// into the local database. Holds no profile data.
|
||||||
|
pub struct ProfileStore {
|
||||||
|
in_flight: RefCell<HashSet<PublicKey>>,
|
||||||
|
sender: Sender<PublicKey>,
|
||||||
|
_subscription: Subscription,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ProfileStore {
|
||||||
|
pub fn global(cx: &App) -> Entity<Self>;
|
||||||
|
pub(crate) fn set_global(entity: Entity<Self>, cx: &mut App);
|
||||||
|
/// Queue a batched metadata request; no-op for keys already in flight.
|
||||||
|
pub fn fetch(&self, keys: impl IntoIterator<Item = PublicKey>);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Metadata cache for one panel or screen.
|
||||||
|
pub struct ProfileDirectory {
|
||||||
|
profiles: HashMap<PublicKey, Profile>,
|
||||||
|
pending: RefCell<HashSet<PublicKey>>,
|
||||||
|
tasks: SmallVec<[Task<()>; 2]>,
|
||||||
|
_subscriptions: SmallVec<[Subscription; 1]>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ProfileDirectory {
|
||||||
|
/// Create a directory, following the profile store's events.
|
||||||
|
pub fn new(cx: &mut App) -> Entity<Self>;
|
||||||
|
/// Resolve keys from the local database, queue the rest for fetch.
|
||||||
|
pub fn track(&mut self, keys: impl IntoIterator<Item = PublicKey>, cx: &mut Context<Self>);
|
||||||
|
/// The profile for a key, or a placeholder that queues a fetch.
|
||||||
|
pub fn profile(&self, public_key: &PublicKey, cx: &App) -> Profile;
|
||||||
|
/// Re-query tracked/pending keys named by a `ProfileEvent`.
|
||||||
|
fn refresh(&mut self, authors: &[PublicKey], cx: &mut Context<Self>);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Panel wiring:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub struct IssuesView {
|
||||||
|
// ...
|
||||||
|
directory: Entity<ProfileDirectory>,
|
||||||
|
}
|
||||||
|
|
||||||
|
// constructor — no observe wiring; render-time reads of the directory are
|
||||||
|
// tracked automatically and `cx.notify()` on it invalidates the panel.
|
||||||
|
let directory = ProfileDirectory::new(cx);
|
||||||
|
```
|
||||||
|
|
||||||
|
Behavior details carried over from coop's `Directory`:
|
||||||
|
|
||||||
|
- The `pending` set guards the window between `track`/`profile` and the
|
||||||
|
database task finishing, so events arriving mid-query are still accepted.
|
||||||
|
- Inserts never overwrite data that already arrived from an event
|
||||||
|
(insert-if-absent on bulk load).
|
||||||
|
- `refresh` removes a pending key only when the database produced a profile,
|
||||||
|
so keys that exist on no relay stay pending and never loop into refetches;
|
||||||
|
`in_flight` in the store is released on every `Updated` emission.
|
||||||
|
|
||||||
|
## Where directories live
|
||||||
|
|
||||||
|
| Surface | File | Keys tracked | When |
|
||||||
|
| --- | --- | --- | --- |
|
||||||
|
| `SidebarPanel` | `views/sidebar/mod.rs` | current user, local repo owners | on `SignerChanged`, on local repos update |
|
||||||
|
| `RepoListView` | `views/repo_list.rs` | announcement owners | when cards rebuild |
|
||||||
|
| `RepoDetailView` | `views/repo/mod.rs`, `repo/about.rs` | owner, maintainers, binding owners | on announcement/sync; about dialog captures the handle |
|
||||||
|
| `IssuesView` | `views/issues/mod.rs` | issue authors, repo owner (tab title) | in `rebuild` |
|
||||||
|
| `IssueDetailView` | `views/issues/detail.rs` | author, discussion participants | on store events; passed to `discussion::` helpers |
|
||||||
|
| `PullRequestsView` | `views/pull_requests/mod.rs` | PR authors, repo owner | in `rebuild` |
|
||||||
|
| `PullRequestDetailView` | `views/pull_requests/detail.rs` | author, participants | on store events |
|
||||||
|
| `NewPullRequestView`, `SendPatchView`, `CommitDiffView` | respective files | repo owner (tab title only) | on construction |
|
||||||
|
| `InboxView` | `views/inbox.rs` | activity authors | in rows rebuild |
|
||||||
|
|
||||||
|
Helpers that stop reading the global and take `&Entity<ProfileDirectory>`
|
||||||
|
instead — every caller has `&mut self` or captures the handle:
|
||||||
|
|
||||||
|
- `repo_tab_avatar(store, cx)` in `views/mod.rs` (all `Panel::title` impls)
|
||||||
|
- `discussion::sidebar_section`, `discussion::comments_section`
|
||||||
|
- `about::maintainers` via `open_about_dialog` (dialog closure captures the
|
||||||
|
entity handle, same as coop's device-registry notification)
|
||||||
|
- `bound_repo_label` in `views/repo/mod.rs`
|
||||||
|
- `sub_activity` in `views/inbox.rs`, `local_avatar` in `views/sidebar/mod.rs`
|
||||||
|
|
||||||
|
## Migration steps
|
||||||
|
|
||||||
|
1. `signed_state`: introduce `ProfileEvent` + `EventEmitter`, extract the
|
||||||
|
"latest metadata per author" reduction from `apply_authors` into a pure
|
||||||
|
`fn latest_profiles(events: Vec<Event>) -> Vec<Profile>`, and strip
|
||||||
|
`ProfileStore` down to the fetch service (drop `profiles`, `seen`, `load`,
|
||||||
|
`apply_seen`, `apply_authors`, `get`, `WARM_LIMIT`). The batch loop emits
|
||||||
|
`ProfileEvent::Updated` instead of querying; the `BackendEvent::ProfileUpdates`
|
||||||
|
subscription re-emits and releases in-flight keys.
|
||||||
|
2. `signed_state`: add `ProfileDirectory` in the same `profile.rs` and export
|
||||||
|
it from `lib.rs`. `init` keeps creating the global store.
|
||||||
|
3. `workspace`: add a `directory` field to each surface in the table, replace
|
||||||
|
`ProfileStore::global(cx).read(cx).get(...)` with
|
||||||
|
`self.directory.read(cx).profile(...)`, thread the handle through the free
|
||||||
|
helpers, and call `track` from each surface's data-load/rebuild path.
|
||||||
|
4. Delete `ProfileStore::get` last; the compiler walks the remaining call
|
||||||
|
sites. The workspace crate will not compile between steps 1 and 3, so this
|
||||||
|
lands as one change.
|
||||||
|
|
||||||
|
## Edge cases
|
||||||
|
|
||||||
|
- **Relay event during a database query**: covered by the pending set.
|
||||||
|
- **Keys with no profile on relays**: stay pending/in-flight, no retry storm
|
||||||
|
(strictly no worse than today's permanent `seen` dedupe).
|
||||||
|
- **Panel closed**: directory drops → subscription, tasks, and cache drop; no
|
||||||
|
cross-panel notifies.
|
||||||
|
- **Same person on many panels**: each directory keeps its own `Profile`
|
||||||
|
clone. Metadata JSON is small and queries are local index lookups; if this
|
||||||
|
ever shows up, a directory can attach to `RepoStore` later.
|
||||||
|
- **Own profile edits**: publishing flows through `Backend` → pump →
|
||||||
|
`ProfileUpdates` → store event → directories refresh, so no extra channel is
|
||||||
|
needed.
|
||||||
|
- **Dock tab titles**: `Panel::title` receives `&mut self`, so panels pass
|
||||||
|
their own directory to `repo_tab_avatar`; no global needed for chrome.
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
- Unit: `latest_profiles` keeps the newest event per author (pure, no GPUI).
|
||||||
|
- Unit: `ProfileDirectory` pending-set dedupe and insert-if-absent merge via
|
||||||
|
`TestAppContext` (the dock crate already uses GPUI test contexts), seeding
|
||||||
|
the directory through a `#[cfg(test)]` helper rather than the network.
|
||||||
|
- Manual: cold start with an empty database, open explore / repo / issues /
|
||||||
|
inbox, confirm placeholders resolve without interacting with each panel, and
|
||||||
|
that `panel.cached` tabs still repaint on profile arrival.
|
||||||
|
|
||||||
|
## Alternatives considered
|
||||||
|
|
||||||
|
- **Attach the directory to `RepoStore`** instead of panels: repo surfaces
|
||||||
|
sharing one store would share one directory, but `RepoStore` instances are
|
||||||
|
created per open and sidebar/inbox/explore still need their own; revisit
|
||||||
|
only if duplicated queries ever matter.
|
||||||
|
- **Keep the global store as-is**: read tracking already propagates updates,
|
||||||
|
but the warm load, unbounded cache, and all-panels invalidation remain; does
|
||||||
|
not meet the goals.
|
||||||
Reference in New Issue
Block a user