Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/dev/track-filter-ranking.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,16 @@ The split between `onValueChanged` and `onActivity` matters: value updates drive

## PropertyRanking — The Ranking Engine

`PropertyRanking` maintains a `std::map<RankKey, RankedEntry, std::greater<>>` — a descending-sorted map of all registered tracks. `RankKey` is `(value, arrivalSeq)` where higher value wins and lower `arrivalSeq` breaks ties (earlier arrivals win).
`PropertyRanking` maintains a `std::map<RankKey, RankedEntry, std::greater<>>` — a descending-sorted map of all registered tracks. `RankKey` is `(value, arrivalSeq)` where higher value wins. `arrivalSeq` (`int64_t`) breaks ties: lower value wins.

Two counters manage `arrivalSeq`:

- **Up-counter** (`nextSeqUp_`, non-negative, incrementing): assigned on registration and on every value *increase*. Whoever rose to a given value first holds the lowest Up seq and wins ties at that level — early session join does not grant priority over an established contributor.
- **Down-counter** (`nextSeqDown_`, negative, decrementing): assigned on every value *decrease*. Negative seqs always sort below non-negative ones, so any track that has decreased to a value permanently outranks tracks that registered at or rose to that value.

The intended consequence: a participant who was recently active and has since gone quiet ranks above one that joined and never contributed. Among past-active tracks at the same value, the most recently decreased (most-negative seq) ranks highest. Rising again clears the negative history by stamping a fresh Up seq.

A known side-effect of this two-tier structure: a track that decreased to a value will beat any track that later rose to that same value, even if the riser arrived more recently. This trade-off is acceptable because the property is expected to be a synthetic application metric (not a raw signal), so exact-value ties at active levels are rare in practice.

A parallel `F14FastMap<FullTrackName, RankIndex>` provides O(1) lookup by name and caches each track's integer rank.

Expand Down
11 changes: 8 additions & 3 deletions src/relay/PropertyRanking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void PropertyRanking::registerTrack(
}

uint64_t value = initialPropertyValue.value_or(0);
RankKey key{value, nextSeq_++};
RankKey key{value, nextSeqUp_++};

publisherTrackCount_[publisher.get()]++;
// Update extended threshold since a publisher's track count changed.
Expand Down Expand Up @@ -97,8 +97,13 @@ void PropertyRanking::updateSortValue(const moxygen::FullTrackName& ftn, uint64_
return;
}

// Construct new key after the early-exit check
RankKey newKey{value, oldKey.arrivalSeq};
// Construct new key after the early-exit check.
// Rise: fresh nextSeqUp_++ so whoever reached this level first keeps priority
// (early registration no longer displaces an established talker).
// Fall: nextSeqDown_-- (negative) so past-active tracks rank above
// never-active or just-rising tracks (non-negative) at the same value.
int64_t newSeq = (value > oldKey.value) ? nextSeqUp_++ : nextSeqDown_--;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good for stepping down, but it seems that stepping up will suffer from previous negative (e.g., stepped down) values being preferred over the recently stepped up value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't really know what the "right" behavior is, and to some degree I rationalized it by saying that the relay can make the rules and the application can adapt.

So for a given level, you would see:

Most recently fallen to this level | Most recently started/rose to this level
<--- Higher Pri                                                Lower Pri ---> 

So your best move to be seen is to scream and then start talking in a normal voice. You will beat the normal people who just talk normally all the time.

RankKey newKey{value, newSeq};

// Capture old rank before modifying the map (oldKey disappears after erase)
uint64_t oldRank = getCachedRank(ftn);
Expand Down
12 changes: 9 additions & 3 deletions src/relay/PropertyRanking.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ namespace openmoq::moqx {
* Uses std::greater<> comparator for descending order (highest value first).
*/
struct RankKey {
uint64_t value; // Property value (higher = better rank)
uint64_t arrivalSeq; // Tie-breaker (lower = earlier = wins)
uint64_t value; // Property value (higher = better rank)
int64_t arrivalSeq; // Tie-breaker: lower = wins.
// Non-negative (0,1,2…): assigned on registration or value increase.
// Whoever rose to this value first holds the lowest Up seq and wins.
// Negative (-1,-2,-3…): assigned on value decrease.
// Past-active tracks (negative) always outrank never-active or
// just-rising tracks (non-negative) at the same value.

bool operator<(const RankKey& other) const {
if (value != other.value) {
Expand Down Expand Up @@ -373,7 +378,8 @@ class PropertyRanking {
// map never holds zombie entries.
folly::F14FastMap<moxygen::MoQSession*, size_t> publisherTrackCount_;

uint64_t nextSeq_{0};
int64_t nextSeqUp_{0};
int64_t nextSeqDown_{-1};
uint64_t selectionThreshold_{0};
// Maximum effective window across all publisher-subscribers: max(N + selfTrackCount).
// Used in crossesThreshold() to avoid fast-path exits that would miss updates
Expand Down
71 changes: 71 additions & 0 deletions test/PropertyRankingBaseTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -701,4 +701,75 @@ TEST_F(PropertyRankingBaseTest, SweepIdle_ActiveTracksNotEvicted) {
EXPECT_EQ(group->trackStates.at(ftn("b")), TrackState::Selected);
}

// ---------------------------------------------------------------------------
// Bidirectional seq counter: past-active tracks rank above never-active
// ---------------------------------------------------------------------------

// A joins first and never talks. B joins later, talks, then goes silent.
// Both end up at value=0. B should rank above A because its seq was stamped
// with the down counter (negative) when its value decreased.
TEST_F(PropertyRankingBaseTest, SeqDown_SilentAfterTalking_WinsOverNeverTalked) {
RankingHarness h;
auto sub = makeSession();

h.ranking().registerTrack(ftn("a"), 0, defaultPublisher_); // seqUp=0
h.ranking().registerTrack(ftn("b"), 0, defaultPublisher_); // seqUp=1
h.ranking().updateSortValue(ftn("b"), 100); // B talks; seqUp=2
h.ranking().updateSortValue(ftn("b"), 0); // B goes silent; seqDown=-1

// At value=0: B has seqDown=-1, A has seqUp=0. Lower seq wins → B is rank 0.
h.ranking().addSessionToTopNGroup(1, sub, /*forward=*/true);
EXPECT_EQ(h.selectCount(ftn("b"), sub.get()), 1);
EXPECT_EQ(h.selectCount(ftn("a"), sub.get()), 0);
}

// A and B both talk then go silent. B stops last (seq=-2). B should rank
// above A (seq=-1) because B's seq is more negative.
TEST_F(PropertyRankingBaseTest, SeqDown_MostRecentStopper_WinsAmongStoppers) {
RankingHarness h;
auto sub = makeSession();

h.ranking().registerTrack(ftn("a"), 0, defaultPublisher_);
h.ranking().registerTrack(ftn("b"), 0, defaultPublisher_);
h.ranking().updateSortValue(ftn("a"), 100);
h.ranking().updateSortValue(ftn("b"), 100);
h.ranking().updateSortValue(ftn("a"), 0); // A stops first → seqDown=-1
h.ranking().updateSortValue(ftn("b"), 0); // B stops second → seqDown=-2

// B has seqDown=-2 < A's seqDown=-1 → B is rank 0.
h.ranking().addSessionToTopNGroup(1, sub, /*forward=*/true);
EXPECT_EQ(h.selectCount(ftn("b"), sub.get()), 1);
EXPECT_EQ(h.selectCount(ftn("a"), sub.get()), 0);
}

// A and B both increase to the same value. Seq is preserved on increase,
// so A's earlier registration seq (0) beats B's (1).
TEST_F(PropertyRankingBaseTest, SeqUp_EarlierTalker_WinsAmongActiveTalkers) {
RankingHarness h;
auto sub = makeSession();

h.ranking().registerTrack(ftn("a"), 0, defaultPublisher_); // seqUp=0
h.ranking().registerTrack(ftn("b"), 0, defaultPublisher_); // seqUp=1
h.ranking().updateSortValue(ftn("a"), 100); // seqUp=2
h.ranking().updateSortValue(ftn("b"), 100); // seqUp=3

// A has seqUp=2 < B's seqUp=3 → A rose to 100 first → A is rank 0.
h.ranking().addSessionToTopNGroup(1, sub, /*forward=*/true);
EXPECT_EQ(h.selectCount(ftn("a"), sub.get()), 1);
EXPECT_EQ(h.selectCount(ftn("b"), sub.get()), 0);
}

// Sanity check: with no value updates, earlier registration still wins.
TEST_F(PropertyRankingBaseTest, RegistrationOrder_PreservedNoUpdates) {
RankingHarness h;
auto sub = makeSession();

h.ranking().registerTrack(ftn("a"), 0, defaultPublisher_); // seqUp=0
h.ranking().registerTrack(ftn("b"), 0, defaultPublisher_); // seqUp=1

h.ranking().addSessionToTopNGroup(1, sub, /*forward=*/true);
EXPECT_EQ(h.selectCount(ftn("a"), sub.get()), 1);
EXPECT_EQ(h.selectCount(ftn("b"), sub.get()), 0);
}

} // namespace
Loading