Skip to content
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

Better unread statistics #521

Merged
merged 10 commits into from
Nov 24, 2021
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ jobs:
env:
TEST_USER: ${{ secrets.TEST_USER }}
TEST_PWD: ${{ secrets.TEST_PWD }}
QT_LOGGING_RULES: 'quotient.main.debug=true;quotient.jobs.debug=true'
QT_MESSAGE_PATTERN: '%{time h:mm:ss.zzz}|%{category}|%{if-debug}D%{endif}%{if-info}I%{endif}%{if-warning}W%{endif}%{if-critical}C%{endif}%{if-fatal}F%{endif}|%{message}'
run: |
[[ -z "$TEST_USER" ]] || $VALGRIND build/quotest/quotest "$TEST_USER" "$TEST_PWD" quotest-gha '#quotest:matrix.org' "$QUOTEST_ORIGIN"
timeout-minutes: 5 # quotest is supposed to finish within 3 minutes, actually
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ list(APPEND lib_SRCS
lib/avatar.cpp
lib/uri.cpp
lib/uriresolver.cpp
lib/eventstats.cpp
lib/syncdata.cpp
lib/settings.cpp
lib/networksettings.cpp
Expand Down
98 changes: 98 additions & 0 deletions lib/eventstats.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// SPDX-FileCopyrightText: 2021 Quotient contributors
// SPDX-License-Identifier: LGPL-2.1-or-later

#include "eventstats.h"

using namespace Quotient;

EventStats EventStats::fromRange(const Room* room, const Room::rev_iter_t& from,
const Room::rev_iter_t& to,
const EventStats& init)
{
Q_ASSERT(to <= room->historyEdge());
Q_ASSERT(from >= Room::rev_iter_t(room->syncEdge()));
Q_ASSERT(from <= to);
QElapsedTimer et;
et.start();
const auto result =
accumulate(from, to, init,
[room](EventStats acc, const TimelineItem& ti) {
acc.notableCount += room->isEventNotable(ti);
acc.highlightCount += room->notificationFor(ti).type
== Notification::Highlight;
return acc;
});
if (et.nsecsElapsed() > profilerMinNsecs() / 10)
qCDebug(PROFILER).nospace()
<< "Event statistics collection over index range [" << from->index()
<< "," << (to - 1)->index() << "] took " << et;
return result;
}

EventStats EventStats::fromMarker(const Room* room,
const EventStats::marker_t& marker)
{
const auto s = fromRange(room, marker_t(room->syncEdge()), marker,
{ 0, 0, marker == room->historyEdge() });
Q_ASSERT(s.isValidFor(room, marker));
return s;
}

EventStats EventStats::fromCachedCounters(Omittable<int> notableCount,
Omittable<int> highlightCount)
{
const auto hCount = std::max(0, highlightCount.value_or(0));
if (!notableCount.has_value())
return { 0, hCount, true };
auto nCount = notableCount.value_or(0);
return { std::max(0, nCount), hCount, nCount != -1 };
}

bool EventStats::updateOnMarkerMove(const Room* room, const marker_t& oldMarker,
const marker_t& newMarker)
{
if (newMarker == oldMarker)
return false;

// Double-check consistency between the old marker and the old stats
Q_ASSERT(isValidFor(room, oldMarker));
Q_ASSERT(oldMarker > newMarker);

// A bit of optimisation: only calculate the difference if the marker moved
// less than half the remaining timeline ahead; otherwise, recalculation
// over the remaining timeline will very likely be faster.
if (oldMarker != room->historyEdge()
&& oldMarker - newMarker < newMarker - marker_t(room->syncEdge())) {
const auto removedStats = fromRange(room, newMarker, oldMarker);
Q_ASSERT(notableCount >= removedStats.notableCount
&& highlightCount >= removedStats.highlightCount);
notableCount -= removedStats.notableCount;
highlightCount -= removedStats.highlightCount;
return removedStats.notableCount > 0 || removedStats.highlightCount > 0;
}

const auto newStats = EventStats::fromMarker(room, newMarker);
if (!isEstimate && newStats == *this)
return false;
*this = newStats;
return true;
}

bool EventStats::isValidFor(const Room* room, const marker_t& marker) const
{
const auto markerAtHistoryEdge = marker == room->historyEdge();
// Either markerAtHistoryEdge and isEstimate are in the same state, or it's
// a special case of no notable events and the marker at history edge
// (then isEstimate can assume any value).
return markerAtHistoryEdge == isEstimate
|| (markerAtHistoryEdge && notableCount == 0);
}

QDebug Quotient::operator<<(QDebug dbg, const EventStats& es)
{
QDebugStateSaver _(dbg);
dbg.nospace() << es.notableCount << '/' << es.highlightCount;
if (es.isEstimate)
dbg << " (estimated)";
return dbg;
}
114 changes: 114 additions & 0 deletions lib/eventstats.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// SPDX-FileCopyrightText: 2021 Quotient contributors
// SPDX-License-Identifier: LGPL-2.1-or-later

#pragma once

#include "room.h"

namespace Quotient {

//! \brief Counters of unread events and highlights with a precision flag
//!
//! This structure contains a static snapshot with values of unread counters
//! returned by Room::partiallyReadStats and Room::unreadStats (properties
//! or methods).
//!
//! \note It's just a simple grouping of counters and is not automatically
//! updated from the room as subsequent syncs arrive.
//! \sa Room::unreadStats, Room::partiallyReadStats, Room::isEventNotable
struct EventStats {
Q_GADGET
Q_PROPERTY(qsizetype notableCount MEMBER notableCount CONSTANT)
Q_PROPERTY(qsizetype highlightCount MEMBER highlightCount CONSTANT)
Q_PROPERTY(bool isEstimate MEMBER isEstimate CONSTANT)
public:
//! The number of "notable" events in an events range
//! \sa Room::isEventNotable
qsizetype notableCount = 0;
qsizetype highlightCount = 0;
//! \brief Whether the counter values above are exact
//!
//! This is false when the end marker (m.read receipt or m.fully_read) used
//! to collect the stats points to an event loaded locally and the counters
//! can therefore be calculated exactly using the locally available segment
//! of the timeline; true when the marker points to an event outside of
//! the local timeline (in which case the estimation is made basing on
//! the data supplied by the homeserver as well as counters saved from
//! the previous run of the client).
bool isEstimate = true;

// TODO: replace with = default once C++20 becomes a requirement on clients
bool operator==(const EventStats& rhs) const
{
return notableCount == rhs.notableCount
&& highlightCount == rhs.highlightCount
&& isEstimate == rhs.isEstimate;
}
bool operator!=(const EventStats& rhs) const { return !operator==(rhs); }

//! \brief Check whether the event statistics are empty
//!
//! Empty statistics have notable and highlight counters of zero and
//! isEstimate set to false.
Q_INVOKABLE bool empty() const
{
return notableCount == 0 && !isEstimate && highlightCount == 0;
}

using marker_t = Room::rev_iter_t;

//! \brief Build event statistics on a range of events
//!
//! This is a factory that returns an EventStats instance with counts of
//! notable and highlighted events between \p from and \p to reverse
//! timeline iterators; the \p init parameter allows to override
//! the initial statistics object and start from other values.
static EventStats fromRange(const Room* room, const marker_t& from,
const marker_t& to,
const EventStats& init = { 0, 0, false });

//! \brief Build event statistics on a range from sync edge to marker
//!
//! This is mainly a shortcut for \code
//! <tt>fromRange(room, marker_t(room->syncEdge()), marker)</tt>
//! \endcode except that it also sets isEstimate to true if (and only if)
//! <tt>to == room->historyEdge()</tt>.
static EventStats fromMarker(const Room* room, const marker_t& marker);

//! \brief Loads a statistics object from the cached counters
//!
//! Sets isEstimate to `true` unless both notableCount and highlightCount
//! are equal to -1.
static EventStats fromCachedCounters(Omittable<int> notableCount,
Omittable<int> highlightCount = none);

//! \brief Update statistics when a read marker moves down the timeline
//!
//! Removes events between oldMarker and newMarker from statistics
//! calculation if \p oldMarker points to an existing event in the timeline,
//! or recalculates the statistics entirely if \p oldMarker points
//! to <tt>room->historyEdge()</tt>. Always results in exact statistics
//! (<tt>isEstimate == false</tt>.
//! \param oldMarker Must point correspond to the _current_ statistics
//! isEstimate state, i.e. it should point to
//! <tt>room->historyEdge()</tt> if <tt>isEstimate == true</tt>, or
//! to a valid position within the timeline otherwise
//! \param newMarker Must point to a valid position in the timeline (not to
//! <tt>room->historyEdge()</tt> that is equal to or closer to
//! the sync edge than \p oldMarker
//! \return true if either notableCount or highlightCount changed, or if
//! the statistics was completely recalculated; false otherwise
bool updateOnMarkerMove(const Room* room, const marker_t& oldMarker,
const marker_t& newMarker);

//! \brief Validate the statistics object against the given marker
//!
//! Checks whether the statistics object data are valid for a given marker.
//! No stats recalculation takes place, only isEstimate and zero-ness
//! of notableCount are checked.
bool isValidFor(const Room* room, const marker_t& marker) const;
};

QDebug operator<<(QDebug dbg, const EventStats& es);

}
Loading