Sliding Sync: Only send rooms with updates down sliding sync

[erikjohnston]

Rather than always including all rooms in range.

Also adds a pre-filter to rooms that checks the stream change cache to see if anything might have happened.

Based on #17447

[MadLittleMods]

I understand why we always need to send a room if we NEVER sent it before (even if there aren't any updates) so that the client can get the base of meta info and required_state.

But I'm confused why we always have to send a room that was PREVIOUSLY sent down?

We should clarify this in the comments there.

[erikjohnston]

Have added a comment, but basically PREVIOUSLY indicates there was an update that we haven't sent down yet, so if the room is now within a list range (or a subscription) we should send it down with the missing update.

The other way to look at is the optimisation of "has anything happened in the rooms since from_token" only makes sense for rooms that are in the LIVE state, i.e. those are the rooms that we have sent down all updates for upto from_token.

[MadLittleMods]

Ahh, I think I had my PREVIOUSLY definition in my head a little wrong to just mean sent down before at all. But it actually means we have previously sent the room down and there are updates that we haven't sent down.

Perhaps this would be a little more clear:

if status.status in (
    # The room was never sent down before so the client needs to know
    # about it regardless of any updates.
    HaveSentRoomFlag.NEVER,
    # `PREVIOUSLY` literally means the "room was sent down before *AND*
    # there are updates we haven't sent down" so we already know this
    # room has updates.
    HaveSentRoomFlag.PREVIOUSLY,
):
    rooms_should_send.add(room_id)

[erikjohnston]

Done. I've also added a clause to check (and ignore) LIVE so we can do an assert_never(..) (which means mypy will pick up if we add another flag and don't update this).

Also happy to just take what you suggested as well, though I think the slight ugliness is worth the the exhaustive matching. WDYT?

[MadLittleMods]

Looks great! I agree that it's worth it. I've been wanting to do exhaustive matching stuff for the membership comparisons since I feel like there is too much wiggle room for things to slip through the cracks.

[MadLittleMods]

Ohh, that self.initial being in the def __bool__(self) -> bool: condition is a tricky important detail here. Added a suggestion to add a comment there to point it out.

[MadLittleMods]

Ahh, I think I had my PREVIOUSLY definition in my head a little wrong to just mean sent down before at all. But it actually means we have previously sent the room down and there are updates that we haven't sent down.

Perhaps this would be a little more clear:

if status.status in (
    # The room was never sent down before so the client needs to know
    # about it regardless of any updates.
    HaveSentRoomFlag.NEVER,
    # `PREVIOUSLY` literally means the "room was sent down before *AND*
    # there are updates we haven't sent down" so we already know this
    # room has updates.
    HaveSentRoomFlag.PREVIOUSLY,
):
    rooms_should_send.add(room_id)