perf: add batch variant of tr_torrentStat() (#8100)

* refactor: add [Torrent updateTorrents]

refactor: use updateTorrents in Controller

* refactor: add a batch variant of tr_torrentStat()

* refactor: use batch variant of tr_torrentStat() in GTK details dialog

* refactor: use batch variant of tr_torrentStat() in gtr_confirm_remove()

* refactor: use batch variant of tr_torrentStat() in updateTorrents

* refactor: add Session::find_torrents()

* refactor: remove the raw ptr variant of updateTorrents()

* refactor: remove tr_sessionLock()

* fixup! refactor: add [Torrent updateTorrents]

remove duplicate method declaration

* fix: readability-avoid-const-params-in-decls

* fix: iwyu in transmission.h

* chore: remove an #include that was added in a draft that did not get used

* refactor: use nullptr instead of NULL

gtk/DetailsDialog.cc

@@ -213,18 +213,7 @@ guint DetailsDialog::Impl::last_page_ = 0;
 
 std::vector<tr_torrent*> DetailsDialog::Impl::getTorrents() const
 {
-    std::vector<tr_torrent*> torrents;
-    torrents.reserve(ids_.size());
-
-    for (auto const id : ids_)
-    {
-        if (auto* torrent = core_->find_torrent(id); torrent != nullptr)
-        {
-            torrents.push_back(torrent);
-        }
-    }
-
-    return torrents;
+    return core_->find_torrents(ids_);
 }
 
 /****
@@ -581,14 +570,13 @@ void DetailsDialog::Impl::refreshInfo(std::vector<tr_torrent*> const& torrents)
     Glib::ustring const no_torrent = _("No Torrents Selected");
     Glib::ustring stateString;
     uint64_t sizeWhenDone = 0;
-    std::vector<tr_stat const*> stats;
-    std::vector<tr_torrent_view> infos;
 
-    stats.reserve(torrents.size());
+    auto const stats = tr_torrentStat(std::data(torrents), std::size(torrents));
+
+    std::vector<tr_torrent_view> infos;
     infos.reserve(torrents.size());
     for (auto* const torrent : torrents)
     {
-        stats.push_back(tr_torrentStat(torrent));
         infos.push_back(tr_torrentView(torrent));
     }
 

gtk/Dialogs.cc

@@ -27,20 +27,18 @@ void gtr_confirm_remove(
     std::vector<tr_torrent_id_t> const& torrent_ids,
     bool delete_files)
 {
-    int connected = 0;
-    int incomplete = 0;
     int const count = torrent_ids.size();
-
     if (count == 0)
     {
         return;
     }
 
-    for (auto const id : torrent_ids)
+    int connected = 0;
+    int incomplete = 0;
+    // TODO(c++20) remove `torrents` local when tr_torrentStat() takes a span
+    auto const torrents = core->find_torrents(torrent_ids);
+    for (auto const* stat : tr_torrentStat(std::data(torrents), std::size(torrents)))
     {
-        tr_torrent* tor = core->find_torrent(id);
-        tr_stat const* stat = tr_torrentStat(tor);
-
         if (stat->leftUntilDone != 0)
         {
             ++incomplete;

gtk/Session.cc

@@ -1416,6 +1416,26 @@ size_t Session::Impl::get_active_torrent_count() const
     return activeCount;
 }
 
+std::vector<tr_torrent*> Session::find_torrents(std::vector<tr_torrent_id_t> const& ids) const
+{
+    auto ret = std::vector<tr_torrent*>{};
+
+    if (auto* const session = impl_->get_session())
+    {
+        ret.reserve(std::size(ids));
+
+        for (auto const& id : ids)
+        {
+            if (auto* const tor = tr_torrentFindFromId(session, id))
+            {
+                ret.emplace_back(tor);
+            }
+        }
+    }
+
+    return ret;
+}
+
 tr_torrent* Session::find_torrent(tr_torrent_id_t id) const
 {
     tr_torrent* tor = nullptr;

gtk/Session.h

@@ -72,6 +72,9 @@ public:
 
     tr_torrent* find_torrent(tr_torrent_id_t id) const;
 
+    // TODO(c++20) std::span
+    [[nodiscard]] std::vector<tr_torrent*> find_torrents(std::vector<tr_torrent_id_t> const& ids) const;
+
     transmission::app::FaviconCache<Glib::RefPtr<Gdk::Pixbuf>>& favicon_cache() const;
 
     /******

libtransmission/session.cc

@@ -461,13 +461,6 @@ tr_address tr_session::bind_address(tr_address_type type) const noexcept
 
 // ---
 
-std::unique_lock<std::recursive_mutex> tr_sessionLock(tr_session const* const session)
-{
-    return session->unique_lock();
-}
-
-// ---
-
 tr_variant tr_sessionGetDefaultSettings()
 {
     auto ret = tr_variant::make_map();

libtransmission/torrent.cc

@@ -1399,6 +1399,27 @@ tr_stat const* tr_torrentStat(tr_torrent* const tor)
     return &tor->stats_;
 }
 
+std::vector<tr_stat const*> tr_torrentStat(tr_torrent* const* torrents, size_t n_torrents)
+{
+    auto ret = std::vector<tr_stat const*>{};
+
+    if (n_torrents != 0U)
+    {
+        ret.reserve(n_torrents);
+
+        auto const lock = torrents[0]->unique_lock();
+
+        for (size_t idx = 0U; idx != n_torrents; ++idx)
+        {
+            tr_torrent* const tor = torrents[idx];
+            tor->stats_ = tor->stats();
+            ret.emplace_back(&tor->stats_);
+        }
+    }
+
+    return ret;
+}
+
 // ---
 
 tr_file_view tr_torrentFile(tr_torrent const* tor, tr_file_index_t file)

libtransmission/torrent.h

@@ -1035,6 +1035,7 @@ private:
     friend bool tr_torrentSetMetainfoFromFile(tr_torrent* tor, tr_torrent_metainfo const* metainfo, char const* filename);
     friend tr_file_view tr_torrentFile(tr_torrent const* tor, tr_file_index_t file);
     friend tr_stat const* tr_torrentStat(tr_torrent* tor);
+    friend std::vector<tr_stat const*> tr_torrentStat(tr_torrent* const* torrents, size_t n_torrents);
     friend tr_torrent* tr_torrentNew(tr_ctor* ctor, tr_torrent** setme_duplicate_of);
     friend uint64_t tr_torrentGetBytesLeftToAllocate(tr_torrent const* tor);
     friend void tr_torrentFreeInSessionThread(tr_torrent* tor);

libtransmission/transmission.h

@@ -14,9 +14,9 @@
 #include <cstdint>
 #include <ctime>
 #include <functional>
-#include <mutex>
 #include <string>
 #include <string_view>
+#include <vector>
 
 #include "libtransmission/tr-macros.h"
 
@@ -139,8 +139,6 @@ inline auto constexpr TrHttpServerDefaultBasePath = std::string_view{ "/transmis
 inline auto constexpr TrHttpServerRpcRelativePath = std::string_view{ "rpc" };
 inline auto constexpr TrHttpServerWebRelativePath = std::string_view{ "web/" };
 
-std::unique_lock<std::recursive_mutex> tr_sessionLock(tr_session const* session);
-
 /**
  * Add libtransmission's default settings to the benc dictionary.
  *
@@ -1591,6 +1589,11 @@ struct tr_stat
     second or so to get a new snapshot of the torrent's status. */
 tr_stat const* tr_torrentStat(tr_torrent* torrent);
 
+// Batch version of tr_torrentStat().
+// Prefer calling this over calling the single-torrent version in a loop.
+// TODO(c++20) take a std::span argument
+std::vector<tr_stat const*> tr_torrentStat(tr_torrent* const* torrents, size_t n_torrents);
+
 /** @} */
 
 /** @brief Sanity checker to test that the direction is `TR_UP` or `TR_DOWN` */

macosx/Controller.mm

@@ -2367,20 +2367,16 @@ void onTorrentCompletenessChanged(tr_torrent* tor, tr_completeness status, bool
     BOOL anyCompleted = NO;
     BOOL anyActive = NO;
 
+    [Torrent updateTorrents:self.fTorrents];
+
+    for (Torrent* torrent in self.fTorrents)
     {
-        // avoid having to wait for the same lock multiple times in the same operation
-        auto const lock = tr_sessionLock(self.sessionHandle);
-        for (Torrent* torrent in self.fTorrents)
-        {
-            [torrent update];
+        //pull the upload and download speeds - most consistent by using current stats
+        dlRate += torrent.downloadRate;
+        ulRate += torrent.uploadRate;
 
-            //pull the upload and download speeds - most consistent by using current stats
-            dlRate += torrent.downloadRate;
-            ulRate += torrent.uploadRate;
-
-            anyCompleted |= torrent.finishedSeeding;
-            anyActive |= torrent.active && !torrent.stalled && !torrent.error;
-        }
+        anyCompleted |= torrent.finishedSeeding;
+        anyActive |= torrent.active && !torrent.stalled && !torrent.error;
     }
 
     PowerManager.shared.shouldPreventSleep = anyActive && [self.fDefaults boolForKey:@"SleepPrevent"];
@@ -3817,9 +3813,10 @@ void onTorrentCompletenessChanged(tr_torrent* tor, tr_completeness status, bool
             for (Torrent* torrent in self.fTorrents)
             {
                 torrent.queuePosition = i++;
-                [torrent update];
             }
 
+            [Torrent updateTorrents:self.fTorrents];
+
             //do the drag animation here so that the dragged torrents are the ones that are animated as moving, and not the torrents around them
             [self.fTableView beginUpdates];
 
@@ -5527,10 +5524,7 @@ void onTorrentCompletenessChanged(tr_torrent* tor, tr_completeness status, bool
 
 - (void)rpcUpdateQueue
 {
-    for (Torrent* torrent in self.fTorrents)
-    {
-        [torrent update];
-    }
+    [Torrent updateTorrents:self.fTorrents];
 
     NSSortDescriptor* descriptor = [NSSortDescriptor sortDescriptorWithKey:@"queuePosition" ascending:YES];
     NSArray* descriptors = @[ descriptor ];

macosx/Torrent.h

@@ -35,6 +35,10 @@ extern NSString* const kTorrentDidChangeGroupNotification;
 - (void)getAmountFinished:(float*)tab size:(int)size;
 @property(nonatomic) NSIndexSet* previousFinishedPieces;
 
+// Updates one or more torrents by refreshing their libtransmission stats.
+// Prefer using this batch method when updating many torrents at once.
++ (void)updateTorrents:(NSArray<Torrent*>*)torrents;
+
 - (void)update;
 
 - (void)startTransferIgnoringQueue:(BOOL)ignoreQueue;

macosx/Torrent.mm

@@ -242,19 +242,59 @@ bool trashDataFile(char const* filename, void* /*user_data*/, tr_error* error)
 
 - (void)update
 {
-    //get previous stalled value before update
-    BOOL const wasTransmitting = self.fStat != NULL && self.transmitting;
+    [Torrent updateTorrents:@[ self ]];
+}
 
-    self.fStat = tr_torrentStat(self.fHandle);
-
-    //make sure the "active" filter is updated when transmitting changes
-    if (wasTransmitting != self.transmitting)
++ (void)updateTorrents:(NSArray<Torrent*>*)torrents
+{
+    if (torrents == nil || torrents.count == 0)
     {
-        //posting asynchronously with coalescing to prevent stack overflow on lots of torrents changing state at the same time
-        [NSNotificationQueue.defaultQueue enqueueNotification:[NSNotification notificationWithName:@"UpdateTorrentsState" object:nil]
-                                                 postingStyle:NSPostASAP
-                                                 coalesceMask:NSNotificationCoalescingOnName
-                                                     forModes:nil];
+        return;
+    }
+
+    std::vector<Torrent*> torrent_objects;
+    torrent_objects.reserve(torrents.count);
+
+    std::vector<tr_torrent*> torrent_handles;
+    torrent_handles.reserve(torrents.count);
+
+    std::vector<BOOL> was_transmitting;
+    was_transmitting.reserve(torrents.count);
+
+    for (Torrent* torrent in torrents)
+    {
+        if (torrent == nil || torrent.fHandle == nullptr)
+        {
+            continue;
+        }
+
+        torrent_objects.emplace_back(torrent);
+        torrent_handles.emplace_back(torrent.fHandle);
+        was_transmitting.emplace_back(torrent.fStat != nullptr && torrent.transmitting);
+    }
+
+    if (torrent_handles.empty())
+    {
+        return;
+    }
+
+    auto const stats = tr_torrentStat(torrent_handles.data(), torrent_handles.size());
+
+    // Assign stats and post notifications.
+    for (size_t i = 0, n = torrent_objects.size(); i < n; ++i)
+    {
+        Torrent* const torrent = torrent_objects[i];
+        torrent.fStat = stats[i];
+
+        //make sure the "active" filter is updated when transmitting changes
+        if (was_transmitting[i] != torrent.transmitting)
+        {
+            //posting asynchronously with coalescing to prevent stack overflow on lots of torrents changing state at the same time
+            [NSNotificationQueue.defaultQueue enqueueNotification:[NSNotification notificationWithName:@"UpdateTorrentsState" object:nil]
+                                                     postingStyle:NSPostASAP
+                                                     coalesceMask:NSNotificationCoalescingOnName
+                                                         forModes:nil];
+        }
     }
 }