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

use madvise(MADV_RANDOM) when torrenting mmaped files to reduce disk readahead #7409

Open
wants to merge 1 commit into
base: RC_2_0
Choose a base branch
from

Conversation

alanswanson
Copy link

For #7405. When torrenting (random_access) files, reduce excessive disk IO readahead with mmaped files by using MADV_RANDOM and hence matching normal file access using POSIX_FADV_RANDOM.

Could cause more IO calls though as unfortunately there's no reduced read ahead option to madvise.

@arvidn
Copy link
Owner

arvidn commented May 13, 2023

Have you measured the performance impact of this change?

@arvidn arvidn added this to the 2.0.9 milestone May 13, 2023
@arvidn
Copy link
Owner

arvidn commented May 14, 2023

reading up on the issue, it sounds like you did not observe a clear improvement. Are you confident the excess reading from disk is caused by read-ahead?
I'm a bit hesitant to throw additional flags in without at least some evidence of it improving things (even though it seems very reasonable to hint random access).

I encountered a windows pitfall a long time ago, relating to a flag like this: https://blog.libtorrent.org/2012/05/windows-disk-cache/

@alanswanson
Copy link
Author

No change for my or original reporters database64128 usage. Probably depends on number active torrents, upload speed and free memory.

Obviously shouldn't affect Windows without madvise while on Linux only disables the readahead with no other effects but mmap readahead has a miss counter to backoff readahead anyway. So probably just close and can reconsider in future if anyone else has problems (and they cannot lower number of readahead pages).

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

Successfully merging this pull request may close these issues.

2 participants