-
Notifications
You must be signed in to change notification settings - Fork 9
Move general append utilities into general index module #467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| module Database.LSMTree.Extras.Index | ||
| ( | ||
| Append (AppendSinglePage, AppendMultiPage), | ||
| append, | ||
| append' | ||
| ) | ||
| where | ||
|
|
||
| import Control.DeepSeq (NFData (rnf)) | ||
| import Control.Monad.ST.Strict (ST) | ||
| import Data.Foldable (toList) | ||
| import Data.Word (Word32) | ||
| import Database.LSMTree.Internal.Chunk (Chunk) | ||
| import Database.LSMTree.Internal.IndexCompactAcc (IndexCompactAcc) | ||
| import qualified Database.LSMTree.Internal.IndexCompactAcc as IndexCompact | ||
| (appendMulti, appendSingle) | ||
| import Database.LSMTree.Internal.IndexOrdinaryAcc (IndexOrdinaryAcc) | ||
| import qualified Database.LSMTree.Internal.IndexOrdinaryAcc as IndexOrdinary | ||
| (appendMulti, appendSingle) | ||
| import Database.LSMTree.Internal.Serialise (SerialisedKey) | ||
|
|
||
| -- | Instruction for appending pages, to be used in conjunction with indexes. | ||
| data Append | ||
| = {-| | ||
| Append a single page that fully comprises one or more key–value pairs. | ||
| -} | ||
| AppendSinglePage | ||
| SerialisedKey -- ^ Minimum key | ||
| SerialisedKey -- ^ Maximum key | ||
| | {-| | ||
| Append multiple pages that together comprise a single key–value pair. | ||
| -} | ||
| AppendMultiPage | ||
| SerialisedKey -- ^ Sole key | ||
| Word32 -- ^ Number of overflow pages | ||
|
|
||
| instance NFData Append where | ||
|
|
||
| rnf (AppendSinglePage minKey maxKey) | ||
| = rnf minKey `seq` rnf maxKey | ||
| rnf (AppendMultiPage key overflowPageCount) | ||
| = rnf key `seq` rnf overflowPageCount | ||
|
|
||
| {-| | ||
| Add information about appended pages to an index under incremental | ||
| construction. | ||
|
|
||
| Internally, 'append' uses 'IndexCompact.appendSingle' and | ||
| 'IndexCompact.appendMulti', and the usage restrictions of those functions | ||
| apply also here. | ||
| -} | ||
| append :: Append -> IndexCompactAcc s -> ST s [Chunk] | ||
| append instruction indexAcc = case instruction of | ||
| AppendSinglePage minKey maxKey | ||
| -> toList <$> IndexCompact.appendSingle (minKey, maxKey) indexAcc | ||
| AppendMultiPage key overflowPageCount | ||
| -> IndexCompact.appendMulti (key, overflowPageCount) indexAcc | ||
|
|
||
| {-| | ||
| A variant of 'append' for ordinary indexes, which is only used temporarily | ||
| until there is a type class of index types. | ||
|
|
||
| Internally, 'append'' uses 'IndexOrdinary.appendSingle' and | ||
| 'IndexOrdinary.appendMulti', and the usage restrictions of those functions | ||
| apply also here. | ||
| -} | ||
| append' :: Append -> IndexOrdinaryAcc s -> ST s [Chunk] | ||
| append' instruction indexAcc = case instruction of | ||
| AppendSinglePage minKey maxKey | ||
| -> toList <$> IndexOrdinary.appendSingle (minKey, maxKey) indexAcc | ||
| AppendMultiPage key overflowPageCount | ||
| -> IndexOrdinary.appendMulti (key, overflowPageCount) indexAcc |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,8 +14,6 @@ module Database.LSMTree.Internal.IndexCompactAcc ( | |
| -- $construction-invariants | ||
| IndexCompactAcc (..) | ||
| , new | ||
| , Append (..) | ||
| , append | ||
| , appendSingle | ||
| , appendMulti | ||
| , unsafeEnd | ||
|
|
@@ -30,11 +28,9 @@ module Database.LSMTree.Internal.IndexCompactAcc ( | |
| import Control.Exception (assert) | ||
| #endif | ||
|
|
||
| import Control.DeepSeq (NFData (..)) | ||
| import Control.Monad (when) | ||
| import Control.Monad.ST.Strict | ||
| import Data.Bit hiding (flipBit) | ||
| import Data.Foldable (toList) | ||
| import Data.List.NonEmpty (NonEmpty) | ||
| import qualified Data.List.NonEmpty as NE | ||
| import Data.Map.Range (Bound (..)) | ||
|
|
@@ -126,26 +122,6 @@ newPinnedMVec64 lenWords = do | |
| setByteArray mba 0 lenWords (0 :: Word64) | ||
| return (VUM.MV_Word64 (VPM.MVector 0 lenWords mba)) | ||
|
|
||
| -- | Min\/max key-info for pages | ||
| data Append = | ||
| -- | One or more keys are in this page, and their values fit within a single | ||
| -- page. | ||
| AppendSinglePage SerialisedKey SerialisedKey | ||
| -- | There is only one key in this page, and it's value does not fit within | ||
| -- a single page. | ||
| | AppendMultiPage SerialisedKey Word32 -- ^ Number of overflow pages | ||
|
|
||
| instance NFData Append where | ||
| rnf (AppendSinglePage kmin kmax) = rnf kmin `seq` rnf kmax | ||
| rnf (AppendMultiPage k nOverflow) = rnf k `seq` rnf nOverflow | ||
|
|
||
| -- | Append a new page entry to a mutable compact index. | ||
| -- | ||
| -- INVARIANTS: see [construction invariants](#construction-invariants). | ||
| append :: Append -> IndexCompactAcc s -> ST s [Chunk] | ||
| append (AppendSinglePage kmin kmax) ica = toList <$> appendSingle (kmin, kmax) ica | ||
| append (AppendMultiPage k n) ica = appendMulti (k, n) ica | ||
|
|
||
|
Comment on lines
-129
to
-148
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a small suggestion for PR review ergonomics: when code is moved to a different module and subsequently changed as well, it's hard to see from the diff what the interesting code changes are. My usual approach is to move the code in one commit, and do the interesting changes in a different commit. Right now we're only talking about a small bit of code, so it's fine and doesn't have to change in this PR, but it might be good to keep in mind for future PRs
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sounds very reasonable to me. I’ll try to remember this approach and use it in future pull requests of mine. Sadly, Git essentially doesn’t track file movement. Well, its manual page calls it right at the beginning “the stupid content tracker”. 😉
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One scenario where Git is smart enough is when a file is first moved and then its contents are changed. The diff will show that the file is renamed, instead of showing that one file was deleted and one new file was created. And the diff of the renamed file just shows the changed lines, not the unchanged lines. But that's about as smart as it gets in my experience
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and it’s only based on heuristics: as soon as the old and the new content differ too much, Git will treat the old and the new file as unrelated, which according to my experience can happen even if they are morally the same file and you as a human can easily see the similarities of the old and the new content.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, now I wonder whether this mishap might have been because of me not actually moving the file but only moving its contents. Does Git perhaps track inode numbers? 🤔
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found the answer: Git does not track inode numbers. It detects renames based on content only. See the documentation of the |
||
| -- | Append a single page to a mutable compact index. | ||
| -- | ||
| -- INVARIANTS: see [construction invariants](#construction-invariants). | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.