1.0.x causes long UI freeze

[GoogleCodeExporter]

Sorry for the non-descriptive title.

I recently released a major version of qbt. This version relies on 1.0.x and I 
have various bug reports coming in that might be related to libtorrent.
This is confirm to be caused by libtorrent 1.0.x

Original bug report: https://github.com/qbittorrent/qBittorrent/issues/2968

The user observes that our mainwindow after a few seconds(after program start) 
it freezes for 8 minutes.

I instructed him to give me a stackstrace during that freeze period.
Here is the stacktrace: 
https://github.com/qbittorrent/qBittorrent/issues/2968#issuecomment-102379418

I couldn't see something obvious from the qbt code.
And from a hunch I provided a test build with 0.16.x (same qbt code).
The user reports that everything works perfectly with it.
I also provided a test build based on 1.0.0 (official build uses 
1.0.4+svn11046). The user reports that the problem exists there too.

If you want some kind of other info I'll try to provide it too.

Original issue reported on code.google.com by hammered...@gmail.com on 16 May 2015 at 8:37

[GoogleCodeExporter]

From what I can tell, qBittorrent's main thread is busy reading alerts. None of 
the libtorrent threads appear to be busy.

You may want to log the alerts you receive and look over your alert mask, 
possibly disabling categories you don't need, or moving the work of handling 
some alerts over to a different thread (or see if handling the most expensive 
alerts can be made more efficient).

When adding a torrent that's completed, you'll get a torrent_finished_alert 
(which it looks like you're processing in this stack trace). It also appears 
that in the handler, you ask for the torrent_info object. before 1.0, the 
torrent info would just be a reference pointing into the copy used by 
libtorrent. This was a potential sharing problem, because the life span of that 
object was not guaranteed. In 1.0 I preserved the API for backwards 
compatibility (but now it's a copy). The new more efficient way of asking for 
the torrent_info object is by calling torrent_handle::torrent_file() which 
returns an intrusive_ptr (which will be a shared_ptr in the future).

Original comment by arvid.no...@gmail.com on 18 May 2015 at 4:59

[GoogleCodeExporter]

btw. it seems like the copying of torrent_info may be a significant cost in 
this case.

Original comment by arvid.no...@gmail.com on 18 May 2015 at 5:00

[GoogleCodeExporter]

Is torrent_handle::torrent_file() cheaper than 
torrent_handle::get_torrent_info() (from 0.16.x)?

We already use torrent_handle::torrent_file(). However I found 2 places(based 
on the stacktrace), where we might actually created a local object of 
torrent_info. I eliminated those places by querying the returned ptr directly.

The user now have this stacktrace: 
https://gist.github.com/clk824/e1e70dee3967a374ebf9

It seems that calling in a for loop this:

int QTorrentHandle::num_files() const
{
    if (!has_metadata())
        return -1;
#if LIBTORRENT_VERSION_NUM < 10000
    return torrent_handle::get_torrent_info().num_files();
#else
    return torrent_handle::torrent_file()->num_files();
#endif
}

results again in slowness. On every iteration num_files is queried as the loop 
condition.

Does torrent_handle::torrent_file() internally create a new copy and assigns to 
a ptr instead of sharing its original?
Am I using the API inefficiently here?

Original comment by hammered...@gmail.com on 19 May 2015 at 3:57

[GoogleCodeExporter]

yes, that call would also be slow. It is actually creating a copy of the 
torrent_info, but on top of that, each call to torrent_file() will ping-pong 
with libtorrent's thread. You should do that as rarely as possible. making a 
copy of the torrent_info object is a stop-gap in 1.0 (done once I realized 
there was a sharing issue with it). In trunk this is fixed to not make a copy.

All libtorrent calls that return something meaningful always block, waiting for 
the libtorrent main thread. Especially under load, this can be quite expensive. 
This is why I've been moving more and more functionality into asynchronous 
calls, where the result comes back as an alert. (perhaps in the future I could 
return std::future).

Original comment by arvid.no...@gmail.com on 19 May 2015 at 11:51

[GoogleCodeExporter]

looking at the stack trace, the the qbittorrent thread is waiting to hear back 
from the libtorrent thread. and libtorrent is busy copying the torrent file.

If you want to provide a function like this, I would suggest you store a copy 
of the torrent file owned by the gui. Or at least cache the number of files and 
some other important details.

Original comment by arvid.no...@gmail.com on 20 May 2015 at 2:01

[GoogleCodeExporter]

2 questions:

1. torrent_handle::torrent_file() is expensive because it needs thread 
synchronization not because the construction of the object takes much time, 
correct?

2. Does torrent_status::torrent_file() need thread synchronization too? I 
assume not therefore it should be very cheap.

Original comment by hammered...@gmail.com on 21 May 2015 at 12:35

[GoogleCodeExporter]

both. in 1.0.x there is a copy which is somewhat expensive. in 1.1 (trunk) 
there won't be a copy, but you'll still block, synchronizing with the 
libtorrent thread. The synchronization is also expensive, or it is potentially 
expensive. Under load, the main libtorrent thread may be very busy, and there 
may be a significant latency to hear back from it.

Original comment by arvid.no...@gmail.com on 22 May 2015 at 12:25

[GoogleCodeExporter]

sorry, to be more clear in answering your questions.

torrent_status::torrent_file() does not require synchronization nor a copy (it 
synchronized when you called torrent_handle::status()). In fact, not even 
torrent_handle::status() seems to copy the torrent_info object into 
torrent_status. Which seems a bit suspicious. They should do the same thing, 
whichever is right. I need to look into that.

Original comment by arvid.no...@gmail.com on 22 May 2015 at 12:31