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

File pool does not work as intended #7778

Closed
HanabishiRecca opened this issue Oct 28, 2024 · 5 comments
Closed

File pool does not work as intended #7778

HanabishiRecca opened this issue Oct 28, 2024 · 5 comments

Comments

@HanabishiRecca
Copy link
Contributor

HanabishiRecca commented Oct 28, 2024

I noticed strange behavior of file descriptors. Opened files stale indefinitely, regardless of the file pool size.

Some investigation with TRACE_FILE_VIEW_POOL enabled shows that libtorrent never rotates any descriptors in the pool, except constantly reusing the last one when the pool is filled up.

135805188216512 opening file: (18, 12)
135805188216512 opening file: (18, 6)
135805188216512 opening file: (0, 0)
135805188216512 opening file: (18, 10)
135805188216512 opening file: (18, 8)
135805188216512 opening file: (18, 4)
135805188216512 opening file: (18, 72)
135805188216512 opening file: (18, 73)
135805188216512 opening file: (18, 74)
135805188216512 removing: (18, 74)
135805188216512 opening file: (18, 47)
135805188216512 removing: (18, 47)
135805188216512 opening file: (18, 82)
135805188216512 removing: (18, 82)
135805188216512 opening file: (18, 83)
135805188216512 removing: (18, 83)
135805188216512 opening file: (18, 21)
135805188216512 removing: (18, 21)
135805188216512 opening file: (18, 48)
135805188216512 removing: (18, 48)
135805188216512 opening file: (18, 61)
135805188216512 removing: (18, 61)
135805188216512 opening file: (18, 21)
135805188216512 removing: (18, 21)
135805188216512 opening file: (27, 0)

Obviously, this is not how it intended to work, and actually could lead to severe performance degradation.

I assume this thing does not sort files as expected.

// look up files by least recently used
mi::sequenced<>

Replacing backs with fronts inside the file_view_pool::remove_oldest function seems to mitigate the issue.

--- a/src/file_view_pool.cpp
+++ b/src/file_view_pool.cpp
@@ -351,11 +351,11 @@ namespace libtorrent { namespace aux {
 
 #if TRACE_FILE_VIEW_POOL
 		std::cout << std::this_thread::get_id() << " removing: ("
-			<< lru_view.back().key.first << ", " << lru_view.back().key.second << ")\n";
+			<< lru_view.front().key.first << ", " << lru_view.front().key.second << ")\n";
 #endif
 
-		auto mapping = std::move(lru_view.back().mapping);
-		lru_view.pop_back();
+		auto mapping = std::move(lru_view.front().mapping);
+		lru_view.pop_front();
 
 		// closing a file may be long running operation (mac os x)
 		// let the caller destruct it once it has released the mutex

I don't know how vaild this fix is, but it does make the pool to actually rotate.

125017016645312 opening file: (3, 371)
125017016645312 opening file: (3, 660)
125017016645312 opening file: (3, 129)
125017016645312 opening file: (8, 258)
125017016645312 opening file: (3, 647)
125017016645312 opening file: (22, 0)
125017016645312 opening file: (3, 474)
125017016645312 opening file: (3, 640)
125017016645312 opening file: (3, 304)
125017016645312 removing: (3, 640)
125017016645312 opening file: (3, 534)
125017016645312 removing: (3, 371)
125017016645312 opening file: (3, 556)
125017016645312 removing: (3, 534)
125017016645312 opening file: (3, 495)
125017016645312 removing: (3, 660)
125017016645312 opening file: (3, 603)
125017016645312 removing: (3, 474)
125017016645312 opening file: (3, 343)
125017016645312 removing: (3, 556)
125017016645312 opening file: (3, 639)
125017016645312 removing: (3, 129)
125017016645312 opening file: (3, 544)
125017016645312 removing: (3, 639)
125017016645312 opening file: (3, 375)
125017016645312 removing: (3, 343)
125017016645312 opening file: (3, 77)
125017016645312 removing: (3, 647)
@arvidn
Copy link
Owner

arvidn commented Oct 29, 2024

Thanks for an excellent report. I will investigate!

@arvidn
Copy link
Owner

arvidn commented Oct 29, 2024

addressed here: #7779

@KyleSanderson
Copy link

I'm guessing this doesn't impact lt1.2? There's some weird fluttering going on, I haven't traced it yet though

@HanabishiRecca
Copy link
Contributor Author

No, 1.2 has completely different implementation.

@Toms200
Copy link

Toms200 commented Dec 19, 2024

I get strange behaviour of torrents not uploading after being active for an extended period. I uploaded 500GB from 416 torrents between 24-48 hours, and many torrents became a standstill. I upgraded the version from 5.0.2 to 5.0.3 of qBittorrent and with the fresh start active torrents went from a few to around 15.

FIY: DC++ has a similar problem of uploads becoming a standstill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants