The chat column is more expensive than it should be right now. In particular:
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Open | None | T7569 Support Feed/Notifications in Conpherence | ||
| Resolved | btrahan | T7565 Ship Conpherence v2 | ||
| Resolved | btrahan | T7572 Ship Durable Chat Column | ||
| Resolved | btrahan | T7574 Conpherence v2 Infrastructure | ||
| Resolved | epriestley | T7708 Improve performance of chat column | ||
| Resolved | epriestley | T7707 Implement a read-through cache layer | ||
| Resolved | None | T7879 Profile pictures should have policy settings forced to public/most-open | ||
| Resolved | epriestley | T5258 Memes use older image transform code without modern error handling | ||
| Restricted Maniphest Task | ||||
| Resolved | None | T4406 Allow for 100x100 images in profile and projects | ||
| Resolved | None | T4724 Some images appear with the wrong aspect ratio |
The column view itself looks has some awkward semantics around Remarkup rendering (multiple redundant fetches for @mentions, for example).
I'm not really seeing this. e.g. cache_markupcache has one query against it?
I think handle pools / more efficiently fetching handles would be a win here; we definitely end up fetching the same handle (and supporting data like calendar events) a few times across the N threads in my test data. Not sure if we want to start deploying that everywhere?
I believe the attached two diffs address the other items. On the building Calendar / Files bullet I only saw that in update paths (D12760).
Let me know what else I can fix here! :D
Pretty sure T7707 is what's left here.
I did try to do some batch loading of objects for PhabricatorObjectRemarkupRule but after a few hours I don't have anything working and I kind of want to put this code in a blender so I should at least take a break. T7707 should make that cost trivial so long as the cache is hit so I'm just going to de-prioritize this indefinitely unless you tell me otherwise. :D
The column view itself looks has some awkward semantics around Remarkup rendering (multiple redundant fetches for @mentions, for example).
Also didn't do anything for this last bit; wasn't sure what was wrong.
I think we're in not-bad shape here now and nothing left is Conpherence-speific or concerning to me from a "we're going to burn everything down" point of view (the multiple extra round trips were the big thing I was a bit worried about). I'm just going to close this in favor of T7707.
One simple thing we might still consider is loading 50 or 25 recent messages by default instead of 100, since it looks like the cost for the column is pretty proportionate to the number of messages we render now and it's probably rare to scroll back through more than 25 messages.
The @mention thing is pretty closely related to ObjectRemarkupRule batching and I didn't see a straightforward way to improve it after poking at it for a bit either. I have some ideas for how to approach this in a viable way, but am similarly inclined to do T7707 first.