Get or set the file caching mode #340
Conversation
System/Posix/Fcntl.hsc
Outdated
| import System.Posix.Types | ||
|
|
||
| #if !HAVE_POSIX_FALLOCATE | ||
| import System.IO.Error ( ioeSetLocation ) | ||
| import GHC.IO.Exception ( unsupportedOperation ) | ||
| #endif | ||
|
|
||
| #ifndef darwin_HOST_OS |
There was a problem hiding this comment.
Why is this check performed on darwin?
ghc-internals does this:
#if !defined(mingw32_HOST_OS) && !defined(javascript_HOST_ARCH)This already suggests it doesn't work with the JS backend. Further, I'd like to avoid platform ifdefs in unix. We can use configure.ac to check, e.g.:
- https://github.com/filebench/filebench/blob/22620e602cbbebad90c0bd041896ebccf70dbf5f/configure.ac#L414-L432
- or https://browse.dgit.debian.org/evolution-data-server.git/plain/libdb/dist/configure.ac?id=1a1b76736a541165dc607de9ccf20d3ad7017990
In the latter case, I think they check if it has an effect... we probably don't need to go that far.
There was a problem hiding this comment.
Makes sense -- I've modelled it after filebench's approach, but made it fit the style of the unix/configure.ac file
I'm not very familiar with autoconf, so let me know if anything is off
There was a problem hiding this comment.
The darwin check was performed like that because we can't set O_DIRECT on OSX -- we have to set the F_NOCACHE flag instead. But the check is rewritten now to case on HAVE_O_DIRECT and HAVE_F_NOCACHE
There was a problem hiding this comment.
( I actually forgot to change the platform ifdef in the imports, but it should now be removed there as well )
|
We need to restore the github workflows to see if JS/wasm backends function. If you don't want to do that, I'll do it later this week. |
| -- Throws 'IOError' (\"unsupported operation\") if platform does not support | ||
| -- reading the cache mode. | ||
| -- | ||
| -- (use @#if HAVE_O_DIRECT@ CPP guard to detect availability). |
There was a problem hiding this comment.
Is this macro available to a user though?
There was a problem hiding this comment.
I'd prefer the documentation to say so explicitly, otherwise people will be left wondering who defines them.
| -- writes are minimised or otherwise eliminated. If the cache mode is 'True', | ||
| -- then cache effects occur like normal. | ||
| -- | ||
| -- On Linux, FreeBSD, and NetBSD this checks whether the @O_DIRECT@ file flag is |
There was a problem hiding this comment.
Is OpenBSD different from other *BSD systems with regards to this?
There was a problem hiding this comment.
From what I can tell, OpenBSD has neither an O_DIRECT file flag or F_NOCACHE fcntl flag.
There was a problem hiding this comment.
There is no POSIX standard for direct IO, so probably not all BSD variants support it, but at least FreeBSD and NetBSD explicitly do
|
Can you rebase against master? |
Will do |
jorisdral
force-pushed
the
jdral/issue-322
branch
from
4b39741 to
b648df3
Compare
| -- If the cache mode is 'False', then cache effects for file system reads and | ||
| -- writes are minimised or otherwise eliminated. If the cache mode is 'True', | ||
| -- then cache effects occur like normal. |
There was a problem hiding this comment.
I now realise this is actually describing the reverse of what it is doing 🤦 This happened because of renaming: readFcntlNoCache -> fileGetCaching, writeFcntlNoCache -> fileSetCaching
There was a problem hiding this comment.
I could rename the functions to fileSetNoCaching/fileGetNoCaching, or invert the interpretation of the boolean argument. Preferences?
There was a problem hiding this comment.
fileSetNoCaching feels like unless, which always causes me brain aneurysm.
jorisdral
force-pushed
the
jdral/issue-322
branch
from
8725f7c to
9d4c1eb
Compare
|
All CI jobs except OpenBSD seem to pass (seems like a problem with the job/runner, not the code?). I can squash the commits into one if the changes are okay to merge |
| -- this sets the @F_NOCACHE@ @fcntl@ flag. | ||
| -- | ||
| -- Throws 'IOError' (\"unsupported operation\") if platform does not support | ||
| -- reading the cache mode. |
Resolves #322