Update disk.hpp - Flush disk cache instantly to avoid stuck in Windows #226
Conversation
Avoid stuck in windows disc cache
zhaoyangwx
changed the title
|
GitHub has a new process that requires a team member to trigger Actions. They are now running. |
src/disk.hpp
Outdated
| @@ -201,6 +206,9 @@ struct FileDisk { | |||
| } | |||
| amtwritten = | |||
| ::fwrite(reinterpret_cast<const char *>(memcache), sizeof(uint8_t), length, f_); | |||
| #ifdef _WIN32 | |||
| _commit(_fileno(f_)); | |||
There was a problem hiding this comment.
it's not obvious that this is an improvement. This will block until the bytes have made it to the disk, right?
Could you provide some explanation of the symptom you're experiencing that this solves?
Ideally it would be explained in a comment here as well, to let future readers of the code understand why this call is here
There was a problem hiding this comment.
The symptom is seen in Chia-Network/chia-blockchain#2337
There was a problem hiding this comment.
could you provide measurements demonstrating the improvement with this patch?
There was a problem hiding this comment.
the reason I feel cautious about this is that you really want disk writes to happen in parallel with your program executing, which means sticking the data in kernel buffers that are flushed to disk at some later time, while the program keeps running. As far as I can tell, this will effectively stop the program to wait for the data to make it all the way down to the platter (presumably surviving a power outage).
Generally, you save a lot of time to keep running instead.
There was a problem hiding this comment.
the documentation doesn't actually say whether the call blocks until the data has been flushed or not.
| #ifdef _WIN32 | ||
| // Flush Windows Cache after few cycles | ||
| flush_cyclecount += 1; | ||
| if (flush_cyclecount >= flush_cyclelimit){ |
There was a problem hiding this comment.
This makes more sense. However, if this turns out to give some people worse performance, they really should have a configuration option to disable it or change this limit.
The limit should probably be specified in number-of-bytes written to the file too.
|
'This PR has been flagged as stale due to no activity for over 60 |
Avoid stuck in windows disk cache when plotting in parallel using a bunch of disks as cache respectively for each task.