Use streaming API for file operations #175
Conversation
joaosreis
added this to the
Milestone 3 : Development of canonical ledger state tooling milestone
There was a problem hiding this comment.
Pull request overview
This PR refactors file operations in the SCLS utility to use a streaming API approach instead of eagerly loading data into memory. The changes eliminate the need to load entire namespace data into memory during merge and extract operations, which should improve memory efficiency for large files. Documentation has also been added to clarify the purpose of each public function.
Key changes:
- Replaced eager loading (
S.toList_) with direct streaming operations usingnamespacedData - Introduced
withNamespaceHandleshelper to manage file handles during merge operations - Added comprehensive module and function-level documentation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| files | ||
| -- Open file handles for each namespace's files, execute the given action, | ||
| -- and ensure all handles are closed afterwards. | ||
| withNamespaceHandles :: Map.Map Namespace [FilePath] -> ([(Namespace, [Handle])] -> IO a) -> IO a |
There was a problem hiding this comment.
The only problem with this approach is that we do open all the handles simultaneously it means that there is an upper bound limit on the number of the namespaces we can handle.
I think it's ok but worth adding a comment
| bracket | ||
| ( Map.foldrWithKey | ||
| ( \ns files acc -> do | ||
| handles <- mapM (\file -> openFile file ReadMode) files |
There was a problem hiding this comment.
We will leak handles in case if an exceptional will arrive in the middle when we opened some handles. There are two ways to resolve that:
- build nester brackets with using foldr so every bracket will be closed
- use bracketOnError to allocate reasources and close if any error happened (keeping only a single bracket).
| (pure []) | ||
| nsToFiles | ||
| ) | ||
| (mapM_ (mapM_ hClose . snd)) |
There was a problem hiding this comment.
I believe that Handle uses MVar inside so hClose may be interruptible it means that if asynchronous exception arrives we may close only some of the handles. Having something like hClose x1 finally hClose x2 ... is cleaner way to do it (or mask uninterruptible (I prefer not doing that))
Implement a streaming API approach for merging and extracting files. Additionally, add documentation to clarify the functionality of file manipulation utilities.