Leverage emulated sector size on Windows to decrease usage of the interface bandwidth with SSD #2597
Conversation
nazar-pc
force-pushed
the
fix-reads-during-proving
branch
from
f02e99e to
b9aea56
Compare
nazar-pc
force-pushed
the
use-emulated-sector-size-windows
branch
from
ab7b8a3 to
229423e
Compare
nazar-pc
force-pushed
the
use-emulated-sector-size-windows
branch
from
229423e to
1da9b62
Compare
| // While buffer above is allocated with granularity of `MAX_DISK_SECTOR_SIZE`, reads are | ||
| // done with granularity of physical sector size |
There was a problem hiding this comment.
Perhaps the buffer can be just Vec<u8> and allocate according to bytes_to_read + offset_in_buffer, non-blocker.
There was a problem hiding this comment.
Buffer's memory needs to be aligned properly, this is why buffer is allocated to contain arrays of 4096 bytes, forcing memory alignment to 4096 bytes. By extension anything that is aligned to 4096 bytes is also aligned to 512 bytes.
| // Physical sector size on many SSDs is smaller than 4096 and should improve performance | ||
| let physical_sector_size = if read_file.read_at(&mut [0; 512], 0).is_ok() { |
There was a problem hiding this comment.
Is this the proper way to determine if the sector size is smaller than 4096? from the doc of read_at it didn't mention in which case it will fail
There was a problem hiding this comment.
As mentioned in the PR description this is a good enough way (from my point of view) to do that.
from the doc of read_at it didn't mention in which case it will fail
First of all this is a different read_at, second the actual requirement comes from FILE_FLAG_NO_BUFFERING we are using (see implementation of advise_unbuffered): https://learn.microsoft.com/en-us/windows/win32/fileio/file-buffering#alignment-and-file-access-requirements
…erface bandwidth with SSD
nazar-pc
force-pushed
the
use-emulated-sector-size-windows
branch
from
1da9b62 to
6819532
Compare
|
Made offset also 512 bytes, to be sure we are checking it properly (offset 0 happens to be aligned with anything, so may have resulted in false-positives) |
While SSDs will not deal with 512 bytes internally and already report that "physical sector" (not really a thing in SSDs, but anyway) is 4096 bytes, most of them still support 512 bytes and some odd controllers even expose 512 bytes as physical, not just logical.
Since many SSDs do support dealing with 512 bytes, my assumption is that it is still beneficial to take advantage of because during operations like reading 32 byte chunks for proving we can save interface bandwidth by transferring data aligned to 512 bytes rather than 4096 (for 4096 we are wasting over 99% of bandwidth when retrieving chunks).
Farmers are currently testing this, but from my personal tests this also reduces disk read bandwidth during farming a bit (~10%), which can also be helpful for those farmers that put a lot of SSDs into JBOD and have essentially shared bandwidth for multiple drives.
Instead of checking "sector" size using OS-specific APIs we just attempt a single read of 512 bytes and it will fail if disk doesn't support it. Given the fact that most SSDs expose either 512 or 4096 this is sufficient.
Code contributor checklist: