storing of info_hash in stats_alert

[GoogleCodeExporter]

I'm trying to optimize qbittorrent a bit.

1/9 time of UI thread in qbittorrent is spend on handling of alerts.

https://cloud.githubusercontent.com/assets/286877/4605759/e00bcd5c-51f8-11e4-908
b-10edc9150a11.png

The slowest alert to handle is stats_alert. As you can see from the profile 
this is because of a calling torrent_handle::info_hash(). The calling of 
torrent_handle::info_hash() is needed because qbittorrent needs an immutable 
key for QHash (unordered_map from Qt).

As I understand torrent_handle is not suitable for using as a key for QHash 
because torrent_handle use weak_ptr, so it could change its value (and 
hash_code) asynchronously.

One of the simplest fixes I can think out is to store info_hash in stats_alert 
as some other alerts do. For example state_update_alert store info_hash inside 
torrent_status,  and it is great that I don't need to call info_hash() for each 
torrent_status. Looks like info_hash is needed in stats_alert handler anyway: I 
don't see what can I do with stats_alert without knowing info_hash of torrent 
it belongs to.

Original issue reported on code.google.com by vanya...@gmail.com on 12 Oct 2014 at 11:47

[GoogleCodeExporter]

Proposed patch (haven't tested because of issue #683)

ivan@liberty:~/d/libtorrent-git$ cat store-info-hash-in-stats-alert.diff 
diff --git a/include/libtorrent/alert_types.hpp 
b/include/libtorrent/alert_types.hpp
index b272ad9..98ffbae 100644
--- a/include/libtorrent/alert_types.hpp
+++ b/include/libtorrent/alert_types.hpp
@@ -1604,8 +1604,8 @@ namespace libtorrent
        struct TORRENT_EXPORT stats_alert: torrent_alert
        {
                // internal
-               stats_alert(torrent_handle const& h, int interval
-                       , stat const& s);
+               stats_alert(torrent_handle const& h, sha1_hash const& info_hash
+                       , int interval, stat const& s);

                TORRENT_DEFINE_ALERT(stats_alert, 57);

@@ -1627,6 +1627,8 @@ namespace libtorrent
                        num_channels
                };

+               sha1_hash info_hash;
+
                // an array of samples. The enum describes what each sample is a
                // measurement of. All of these are raw, and not smoothing is performed.
                int transferred[num_channels];
diff --git a/src/alert.cpp b/src/alert.cpp
index 359e876..7e7c5a7 100644
--- a/src/alert.cpp
+++ b/src/alert.cpp
@@ -389,9 +389,10 @@ namespace libtorrent {
                return msg;
        }

-       stats_alert::stats_alert(torrent_handle const& h, int in
+       stats_alert::stats_alert(torrent_handle const& h, sha1_hash const& ih, 
int in
                , stat const& s)
                : torrent_alert(h)
+               , info_hash(ih)
                , interval(in)
        {
 #ifndef TORRENT_DISABLE_FULL_STATS
diff --git a/src/torrent.cpp b/src/torrent.cpp
index 28751d9..342cff4 100644
--- a/src/torrent.cpp
+++ b/src/torrent.cpp
@@ -9636,7 +9636,7 @@ namespace libtorrent
                        }
                }
                if (m_ses.alerts().should_post<stats_alert>())
-                       m_ses.alerts().post_alert(stats_alert(get_handle(), 
tick_interval_ms, m_stat));
+                       m_ses.alerts().post_alert(stats_alert(get_handle(), 
info_hash(), tick_interval_ms, m_stat));

                m_total_uploaded += m_stat.last_payload_uploaded();
                m_total_downloaded += m_stat.last_payload_downloaded();

Original comment by vanya...@gmail.com on 12 Oct 2014 at 11:49

[GoogleCodeExporter]

This idea makes sense. I would like to explore a somewhat more general fix 
though. Ideally, calling info_hash() on a torrent_handle would be efficient. 
It's almost possible to just call it from any thread now, but not quite 
(especially the download from URL feature may replace the m_torrent_file which 
would cause a race condition).

it would be nicer to have a performance fix to apply to all torrent_handles 
though.

Original comment by arvid.no...@gmail.com on 12 Oct 2014 at 5:12

[GoogleCodeExporter]

> Ideally, calling info_hash() on a torrent_handle would be efficient.
> it would be nicer to have a performance fix to apply to all torrent_handles 
though.

You are maintainer, so you decide. I will be perfectly happy with any solution 
that is fast. The faster -- the better.

Note, for qbittorrent speed of info_hash() doesn't matter now. As you can see I 
removed calls to info_hash() from all hot path except this one.

Original comment by vanya...@gmail.com on 12 Oct 2014 at 8:59

[GoogleCodeExporter]

could test this patch? It makes torrent_handle::info_hash() not require a 
round-trip to the network thread.

http://dpaste.com/34EZQM9

Original comment by arvid.no...@gmail.com on 9 Nov 2014 at 11:17

[GoogleCodeExporter]

I put this in RC_1_0. So, closing.

Original comment by arvid.no...@gmail.com on 23 Nov 2014 at 2:11

• Changed state: Fixed