Address review comments

* Rename readBlobFile to readBlob
* Change newBlobFile to openBlobFile, also now responsible for opening the
  file, rather than adopting an existing file handle.
* Update comments
* Add specialise pragmas
* Drop unused language pragmas

Author: dcoutts

src/Database/LSMTree/Internal/BlobFile.hs

@@ -1,15 +1,10 @@
-{-# LANGUAGE DeriveAnyClass     #-}
-{-# LANGUAGE DeriveGeneric      #-}
-{-# LANGUAGE DerivingStrategies #-}
-{- HLINT ignore "Use unless" -}
-
 module Database.LSMTree.Internal.BlobFile (
     BlobFile (..)
   , BlobSpan (..)
   , removeReference
   , RemoveFileOnClose (..)
-  , newBlobFile
-  , readBlobFile
+  , openBlobFile
+  , readBlob
   ) where
 
 import           Control.DeepSeq (NFData (..))
@@ -27,7 +22,7 @@ import qualified System.FS.API as FS
 import           System.FS.API (HasFS)
 import qualified System.FS.BlockIO.API as FS
 
--- | An open handle to a file containing blobs.
+-- | A handle to a file containing blobs.
 --
 -- This is a reference counted object. Upon finalisation, the file is closed
 -- and deleted.
@@ -51,6 +46,7 @@ data BlobSpan = BlobSpan {
 instance NFData BlobSpan where
   rnf (BlobSpan a b) = rnf a `seq` rnf b
 
+{-# INLINE removeReference #-}
 removeReference ::
      (MonadMask m, PrimMonad m)
   => BlobFile m h
@@ -63,37 +59,41 @@ removeReference BlobFile{blobFileRefCounter} =
 data RemoveFileOnClose = RemoveFileOnClose | DoNotRemoveFileOnClose
   deriving stock Eq
 
--- | Adopt an existing open file handle to make a 'BlobFile'. The file must at
--- least be open for reading (but may or may not be open for writing).
+-- | Open the given file to make a 'BlobFile'. The finaliser will close and
+-- delete the file.
 --
--- The finaliser will close and delete the file.
+-- TODO: Temporarily we have a 'RemoveFileOnClose' flag, which can be removed
+-- once 'Run' no longer needs it, when snapshots are implemented.
 --
-newBlobFile ::
+{-# SPECIALISE openBlobFile :: HasFS IO h -> FS.FsPath -> FS.OpenMode -> RemoveFileOnClose -> IO (BlobFile IO h) #-}
+openBlobFile ::
      PrimMonad m
   => HasFS m h
+  -> FS.FsPath
+  -> FS.OpenMode
   -> RemoveFileOnClose
-  -> FS.Handle h
   -> m (BlobFile m h)
-newBlobFile fs r blobFileHandle = do
+openBlobFile fs path mode remove = do
+    blobFileHandle <- FS.hOpen fs path mode
     let finaliser = do
           FS.hClose fs blobFileHandle
-          unless (r == DoNotRemoveFileOnClose) $
+          unless (remove == DoNotRemoveFileOnClose) $
             FS.removeFile fs (FS.handlePath blobFileHandle)
     blobFileRefCounter <- RC.mkRefCounter1 (Just finaliser)
     return BlobFile {
       blobFileHandle,
       blobFileRefCounter
     }
 
-{-# SPECIALISE readBlobFile :: HasFS IO h -> BlobFile IO h -> BlobSpan -> IO SerialisedBlob #-}
-readBlobFile ::
+{-# SPECIALISE readBlob :: HasFS IO h -> BlobFile IO h -> BlobSpan -> IO SerialisedBlob #-}
+readBlob ::
      (MonadThrow m, PrimMonad m)
   => HasFS m h
   -> BlobFile m h
   -> BlobSpan
   -> m SerialisedBlob
-readBlobFile fs BlobFile {blobFileHandle}
-                BlobSpan {blobSpanOffset, blobSpanSize} = do
+readBlob fs BlobFile {blobFileHandle}
+            BlobSpan {blobSpanOffset, blobSpanSize} = do
     let off = FS.AbsOffset blobSpanOffset
         len :: Int
         len = fromIntegral blobSpanSize

src/Database/LSMTree/Internal/BlobRef.hs

@@ -141,7 +141,7 @@ readRawBlobRef ::
   -> RawBlobRef m h
   -> m SerialisedBlob
 readRawBlobRef fs RawBlobRef {rawBlobRefFile, rawBlobRefSpan} =
-    BlobFile.readBlobFile fs rawBlobRefFile rawBlobRefSpan
+    BlobFile.readBlob fs rawBlobRefFile rawBlobRefSpan
 
 {-# SPECIALISE readWeakBlobRef :: HasFS IO h -> WeakBlobRef IO h -> IO SerialisedBlob #-}
 readWeakBlobRef ::
@@ -152,7 +152,7 @@ readWeakBlobRef ::
 readWeakBlobRef fs wref =
     bracket (deRefWeakBlobRef wref) removeReference $
       \StrongBlobRef {strongBlobRefFile, strongBlobRefSpan} ->
-        BlobFile.readBlobFile fs strongBlobRefFile strongBlobRefSpan
+        BlobFile.readBlob fs strongBlobRefFile strongBlobRefSpan
 
 {-# SPECIALISE readWeakBlobRefs :: HasBlockIO IO h -> V.Vector (WeakBlobRef IO h) -> IO (V.Vector SerialisedBlob) #-}
 readWeakBlobRefs ::

src/Database/LSMTree/Internal/Run.hs

@@ -169,7 +169,7 @@ mkWeakBlobRef Run{runBlobFile} blobspan =
 --
 -- TODO: Once snapshots are implemented, files should get removed, but for now
 -- we want to be able to re-open closed runs from disk.
--- TODO: see newBlobFile DoNotRemoveFileOnClose. This can be dropped at the
+-- TODO: see openBlobFile DoNotRemoveFileOnClose. This can be dropped at the
 -- same once when snapshots are implemented.
 close :: (MonadSTM m, MonadMask m, PrimMonad m) => Run m h -> m ()
 close Run {..} = do
@@ -227,8 +227,8 @@ fromMutable runRunDataCaching refCount builder = do
     (runHasFS, runHasBlockIO, runRunFsPaths, runFilter, runIndex, runNumEntries) <-
       Builder.unsafeFinalise (runRunDataCaching == NoCacheRunData) builder
     runKOpsFile <- FS.hOpen runHasFS (runKOpsPath runRunFsPaths) FS.ReadMode
-    runBlobFile <- newBlobFile runHasFS DoNotRemoveFileOnClose
-               =<< FS.hOpen runHasFS (runBlobPath runRunFsPaths) FS.ReadMode
+    runBlobFile <- openBlobFile runHasFS (runBlobPath runRunFsPaths) FS.ReadMode
+                                DoNotRemoveFileOnClose
     setRunDataCaching runHasBlockIO runKOpsFile runRunDataCaching
     rec runRefCounter <- RC.unsafeMkRefCounterN refCount (Just $ close r)
         let !r = Run { .. }
@@ -313,8 +313,8 @@ openFromDisk fs hbio runRunDataCaching runRunFsPaths = do
         =<< readCRC (forRunIndex expectedChecksums) (forRunIndex paths)
 
     runKOpsFile <- FS.hOpen fs (runKOpsPath runRunFsPaths) FS.ReadMode
-    runBlobFile <- newBlobFile fs DoNotRemoveFileOnClose
-               =<< FS.hOpen fs (runBlobPath runRunFsPaths) FS.ReadMode
+    runBlobFile <- openBlobFile fs (runBlobPath runRunFsPaths) FS.ReadMode
+                                DoNotRemoveFileOnClose
     setRunDataCaching hbio runKOpsFile runRunDataCaching
     rec runRefCounter <- RC.unsafeMkRefCounterN (RefCount 1) (Just $ close r)
         let !r = Run

src/Database/LSMTree/Internal/WriteBufferBlobs.hs

@@ -121,9 +121,9 @@ new :: PrimMonad m
 new fs blobFileName = do
     -- Must use read/write mode because we write blobs when adding, but
     -- we can also be asked to retrieve blobs at any time.
-    blobFileHandle <- FS.hOpen fs blobFileName (FS.ReadWriteMode FS.MustBeNew)
+    blobFile <- openBlobFile fs blobFileName (FS.ReadWriteMode FS.MustBeNew)
+                                RemoveFileOnClose
     blobFilePointer <- newFilePointer
-    blobFile <- newBlobFile fs RemoveFileOnClose blobFileHandle
     return WriteBufferBlobs {
       blobFile,
       blobFilePointer