Skip to content

Commit 778ef9d

Browse files
committed
Remove some obsolete TODOs and unused declarations
1 parent 008dbce commit 778ef9d

File tree

9 files changed

+3
-73
lines changed

9 files changed

+3
-73
lines changed

src/Database/LSMTree/Internal/Config.hs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,7 @@ data MergePolicy =
164164
-- This makes it easier for delete operations to disappear on the last
165165
-- level.
166166
MergePolicyLazyLevelling
167-
{- TODO: disabled for now. Would we ever want to provide pure tiering?
168-
| MergePolicyTiering
169-
-}
170-
{- TODO: disabled for now
171-
| MergePolicyLevelling
172-
-}
167+
-- TODO: add other merge policies, like tiering and levelling.
173168
deriving stock (Eq, Show)
174169

175170
instance NFData MergePolicy where
@@ -200,13 +195,6 @@ data WriteBufferAlloc =
200195
-- NOTE: if the sizes of values vary greatly, this can lead to wonky runs on
201196
-- disk, and therefore unpredictable performance.
202197
AllocNumEntries !NumEntries
203-
{- TODO: disabled for now
204-
| -- | Total number of bytes that the write buffer can use.
205-
--
206-
-- The maximum is 4GiB, which should be more than enough for realistic
207-
-- applications.
208-
AllocTotalBytes !Word32
209-
-}
210198
deriving stock (Show, Eq)
211199

212200
instance NFData WriteBufferAlloc where

src/Database/LSMTree/Internal/Lookup.hs

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,7 @@ indexSearches !arena !indexes !kopsFiles !ks !rkixs = V.generateM n $ \i -> do
9797
!k = ks `V.unsafeIndex` kix
9898
!pspan = Index.search k c
9999
!size = pageSpanSize pspan
100-
-- The current allocation strategy is to allocate a new pinned
101-
-- byte array for each 'IOOp'. One optimisation we are planning to
102-
-- do is to use a cache of re-usable buffers, in which case we
103-
-- decrease the GC load. TODO: re-usable buffers.
100+
-- Acquire a reusable buffer
104101
(!off, !buf) <- allocateFromArena arena (getNumPages size * 4096) 4096
105102
pure $! IOOpRead
106103
h
@@ -111,32 +108,6 @@ indexSearches !arena !indexes !kopsFiles !ks !rkixs = V.generateM n $ \i -> do
111108
where
112109
!n = VP.length rkixs
113110

114-
{-
115-
Note [Batched lookups, buffer strategy and restrictions]
116-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
117-
118-
As a first implementation, we use a simple buffering strategy for batched
119-
lookups: allocate fresh buffers, and GC them once they are no longer used. In
120-
a later phase of development, we will look into more elaborate buffer
121-
strategies, for example using a cache of page buffers, which reduces the
122-
number of blocks of memory that we allocate/free.
123-
124-
When we implement a reusable buffer strategy, we will have to take extra care
125-
to copy bytes from raw pages when necessary. 'rawPageLookup' slices serialised
126-
values from pages without copying bytes. If the serialised value survives
127-
after the reusable buffer is returned to the cache, accessing that serialised
128-
value's bytes has undefined behaviour __unless__ we ensure that we copy from
129-
the raw page instead of or in addition to slicing.
130-
131-
There are currently no alignment constraints on the buffers, but there will be
132-
in the future. The plan is to optimise file access by having key\/operation
133-
files opened with the @O_DIRECT@ flag, which requires buffers to be aligned to
134-
the filesystem block size (typically 4096 bytes). We expect to benefit from
135-
this flag in the UTxO use case, because UTxO keys are uniformly distributed
136-
hashes, which means that there is little to no spatial locality when
137-
performing lookups, and so we can skip looking at the page cache.
138-
-}
139-
140111
-- | Value resolve function: what to do when resolving two @Mupdate@s
141112
type ResolveSerialisedValue = SerialisedValue -> SerialisedValue -> SerialisedValue
142113

src/Database/LSMTree/Internal/MergeSchedule.hs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ module Database.LSMTree.Internal.MergeSchedule (
3636
, MergeDebt (..)
3737
, MergeCredits (..)
3838
, supplyCredits
39-
, creditThresholdForLevel
4039
, NominalDebt (..)
4140
, NominalCredits (..)
4241
, nominalDebtAsCredits
@@ -353,7 +352,6 @@ iforLevelM_ lvls k = V.iforM_ lvls $ \i lvl -> k (LevelNo (i + 1)) lvl
353352
--
354353
-- TODO: So far, this is
355354
-- * not considered when creating cursors (also used for range lookups)
356-
-- * never made merge progress on (by supplying credits to it)
357355
-- * never merged into the regular levels
358356
data UnionLevel m h =
359357
NoUnion
@@ -939,10 +937,3 @@ nominalDebtForLevel TableConfig {
939937
confSizeRatio
940938
} ln =
941939
NominalDebt (maxRunSizeTiering (sizeRatioInt confSizeRatio) bufferSize ln)
942-
943-
-- TODO: the thresholds for doing merge work should be different for each level,
944-
-- maybe co-prime?
945-
creditThresholdForLevel :: TableConfig -> LevelNo -> MR.CreditThreshold
946-
creditThresholdForLevel conf (LevelNo _i) =
947-
let AllocNumEntries (NumEntries x) = confWriteBufferAlloc conf
948-
in MR.CreditThreshold (MR.UnspentCredits (MergeCredits x))

src/Database/LSMTree/Internal/Range.hs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ import Control.DeepSeq (NFData (..))
1111
-------------------------------------------------------------------------------}
1212

1313
-- | A range of keys.
14-
--
15-
-- TODO: consider adding key prefixes to the range type.
1614
data Range k =
1715
-- | Inclusive lower bound, exclusive upper bound
1816
FromToExcluding k k

src/Database/LSMTree/Internal/WriteBuffer.hs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ lookups ::
9797
-> V.Vector (Maybe (Entry SerialisedValue BlobSpan))
9898
lookups (WB !m) !ks = V.mapStrict (`Map.lookup` m) ks
9999

100-
-- | TODO: update once blob references are implemented
101100
lookup ::
102101
WriteBuffer
103102
-> SerialisedKey

test/Test/Database/LSMTree/Internal.hs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@
22
{-# LANGUAGE OverloadedStrings #-}
33
{-# LANGUAGE RecordWildCards #-}
44

5-
{- HLINT ignore "Use <=<" -}
6-
7-
-- TODO: generalise tests in this module for IOSim, not just IO. Do this once we
8-
-- add proper support for IOSim for fault testing.
95
module Test.Database.LSMTree.Internal (tests) where
106

117
import Control.Exception

test/Test/Database/LSMTree/Internal/Snapshot/Codec.hs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import Test.Tasty
2727
import Test.Tasty.QuickCheck
2828
import Test.Util.Arbitrary
2929

30-
-- TODO: we should add golden tests for the CBOR encoders. This should prevent
31-
-- accidental breakage in the format.
3230
tests :: TestTree
3331
tests = testGroup "Test.Database.LSMTree.Internal.Snapshot.Codec" [
3432
testGroup "SnapshotVersion" [

test/Test/Database/LSMTree/StateMachine.hs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -768,8 +768,6 @@ data Action' h a where
768768
=> Var h (WrapTable h IO k v b)
769769
-> Portion
770770
-> Act' h R.UnionCredits
771-
-- TODO: we should assert that debt decreases monotonically as credits are
772-
-- supplied.
773771

774772
portionOf :: Portion -> R.UnionDebt -> R.UnionCredits
775773
portionOf (Portion denominator) (R.UnionDebt debt)
@@ -1317,7 +1315,7 @@ instance ( Eq (Class.TableConfig h)
13171315
Interpreter for the model
13181316
-------------------------------------------------------------------------------}
13191317

1320-
-- TODO: there are a bunch of TODO(err) in 'runMode;' on the last argument to
1318+
-- TODO: there are a bunch of TODO(err) in 'runModel' on the last argument to
13211319
-- 'Model.runModelMWithInjectedErrors'. This last argument defines how the model
13221320
-- should respond to injected errors. Since we don't generate injected errors
13231321
-- for most of these actions yet, they are left open. We will fill these in as
@@ -2591,12 +2589,6 @@ data Tag =
25912589
| DeleteExistingSnapshot
25922590
-- | Delete a missing snapshot
25932591
| DeleteMissingSnapshot
2594-
-- | Open a snapshot with the wrong label
2595-
| OpenSnapshotWrongLabel -- TODO: implement
2596-
-- | A merge happened on level @n@
2597-
| MergeOnLevel Int -- TODO: implement
2598-
-- | A table was closed twice
2599-
| TableCloseTwice String -- TODO: implement
26002592
-- | A corrupted snapshot was created successfully
26012593
| CreateSnapshotCorrupted R.SnapshotName
26022594
-- | An /un/corrupted snapshot was created successfully
@@ -2718,8 +2710,6 @@ data FinalTag =
27182710
| ActionSuccess String
27192711
-- | Which actions failed
27202712
| ActionFail String Model.Err
2721-
-- | Total number of flushes
2722-
| NumFlushes String -- TODO: implement
27232713
-- | Number of tables created (new, open or duplicate)
27242714
| NumTables String
27252715
-- | Number of actions on each table

test/Test/Database/LSMTree/StateMachine/Op.hs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ sameOp = go
130130
instance Eq (Op a b) where
131131
(==) = sameOp
132132

133-
-- TODO: parentheses
134133
instance Show (Op a b) where
135134
showsPrec :: Int -> Op a b -> ShowS
136135
showsPrec p = \op -> case op of

0 commit comments

Comments
 (0)