Make it possible to choose the index type

[jeltsch]

This pull requests enables choice of index types via table configurations.

[jorisdral]

The PR is a draft, but I thought I'd write down my thoughts anyway.

Type parameter i or j

We parameterise over j (index accumulator) in a lot of places, where I think i (the index itself) would be nicer conceptually. I don't think it actually complicates the code to have an additional type family like

type family Acc i
type instance Acc IndexOrdinary = IndexOrdinaryAcc
type instance Acc IndexCompact = IndexCompactAcc

Existentially quantifying at the bottom

We can still give this a try to see if it works and whether we think it's nicer to have. I'd be happy to try if you want me to

[jorisdral]

(This is not something we have to address directly in the current PR, we can do it in a separate one)

This is a general comment, but I use addKeyOp as an example. With the inclusion of new index class constraints, we're probably missing specialisations. We should probably add specialisation pragmas for IO and IndexCompactAcc, but also for IO and IndexOrdinaryAcc. https://gitlab.haskell.org/ghc/ghc/-/issues/24359 would allow us to write both specialisations without much reptition as follows:

{-# SPECIALISE addKeyOp @IO @IndexCompactAcc #-}
{-# SPECIALISE addKeyOp @IO @IndexOrdinaryAcc #-}

But this feature is not available yet, so we'd have to repeat multiple lines of specialisation pragmas. We can use CPP to make our lives a little bit easier:

#define spec_addKeyOp(m, j) \
{-# SPECIALISE addKeyOp :: \
     RunBuilder m h j \
  -> SerialisedKey \
  -> Entry SerialisedValue (RawBlobRef m h) \
  -> m () #-}

spec_addKeyOp(IO, IndexOrdinaryAcc)
spec_addKeyOp(IO, IndexCompactAcc)

For small type signatures, it might not be necessary to use CPP, but for large signatures it's probably useful.

[jorisdral]

Whoops! I accidentall changed the last argument, which I should not have

Suggested change

	const
	(resolve (Proxy @v))

This means the ResolveValue constraints also have to be reinstated

[jorisdral]

I'd expect something like the following to also work:

Suggested change

	-- | Create a new index accumulator with a default configuration.
	newWithDefaults :: ST s (j s)
	type IndexAccArgs j :: Type
	new :: IndexAccArgs j -> ST s (j s)

Yes, the args may be different for different indexes, but we can construct these args in a context where we know the concrete index type. For example, we could have something like:

data FencePointerIndex =
    CompactIndex (Maybe (... {- some compact index configuration stuff -})
  | OrdinaryIndex  (Maybe (... {- some ordinary index configuration stuff -})

data SomeFencePointerIndex where
  SomeFencePointerIndex :: forall j. IndexAcc j => Proxy# j -> IndexAccArgs j -> SomeFencePointerIndex

someFencePointerIndex :: TableConfig -> SomeFencePointerIndex
someFencePointerIndex conf = confFencePointerIndex conf of
    CompactIndex mstuff -> 
      let args = case mstuff of
            Nothing -> ... {- use defaults -}
            Just stuff -> ... {- construct args from stuff -}
       in SomeFencePointerIndex (proxy# @IndexCompactAcc) args
    OrdinaryIndex mstuff -> 
      let args = case mstuff of
            Nothing -> ... {- use defaults -}
            Just stuff -> ... {- construct args from stuff -}
       in SomeFencePointerIndex (proxy# @OrdinaryIndex) args

[jorisdral]

I wonder if we have to have explicit constructors for the compact/ordinary cases for the code to use specialised code paths.

Suggested change

	data SomeFencePointerIndex where
	SomeFencePointerIndex :: forall j. IndexAcc j => Proxy# j -> SomeFencePointerIndex
	someFencePointerIndex :: FencePointerIndex -> SomeFencePointerIndex
	someFencePointerIndex CompactIndex = SomeFencePointerIndex (proxy# @IndexCompactAcc)
	someFencePointerIndex OrdinaryIndex = SomeFencePointerIndex (proxy# @IndexOrdinaryAcc)
	data SomeFencePointerIndex where
	SomeFencePointerIndex :: forall j. IndexAcc j => Proxy# j -> SomeFencePointerIndex
	SomeCompactIndex :: Proxy# CompactIndex -> SomeFencePointerIndex
	SomeOrdinaryIndex :: Proxy# OrdinaryIndex -> SomeOrdinaryIndex
	someFencePointerIndex :: FencePointerIndex -> SomeFencePointerIndex
	someFencePointerIndex CompactIndex = SomeCompactIndex (proxy# @IndexCompactAcc)
	someFencePointerIndex OrdinaryIndex = SomeOrdinaryIndex (proxy# @IndexOrdinaryAcc)

Take this example, where the SomeFencePointerIndex case leads to calls to unspecialised functions

{-# SPECIALISE f :: IndexCompactAcc s -> ... #-}
{-# SPECIALISE f :: IndexOrdinaryAcc s -> ... #-}
f :: IndexAcc j => ... {- elided -}

example x = case x of
  SomeFencePointerIndex (_ :: Proxy# j) -> f @j ... -- this is a non-specialised call
  SomeCompactIndex (_ :: Proxy# IndexCompactAcc) -> f @IndexCompactAcc ... -- this is a specialised call
  SomeOrdinaryIndex (_ :: Proxy# IndexOrdinaryAcc) -> f @IndexOrdinaryAcc ... -- this is a specialised call

[jorisdral]

If we add my IndexAccArgs suggestion, then we could do this:

Suggested change

	-> ST (PrimState m) (j (PrimState m))
	-> IndexAccArgs j

[jorisdral]

You already mentioned it through other channels that some type signatures like this one already had the :: on a new line.

grep -E "^( )*::" **.hs tells me there are 54 such signatures. We can change them at some point in a dedicated PR

[jorisdral]

This changes the :: to be on a newline, let's keep it as it was before. I haven't checked whether there are any other similar changes in this PR, but it would be good to revert those too

[jorisdral]

Functions like this one that previously had no class constraints now need SPECIALISE pragmas for IndexCompactAcc and IndexOrdinaryAcc. It would be good to go through all the affected type signatures and add specialisations if necessary

[jorisdral]

All the changes in this module can be reverted, I think

[jeltsch]

Existentially quantifying at the bottom

We can still give this a try to see if it works and whether we think it's nicer to have. I’d be happy to try if you want me to.

Actually, I’m meanwhile eager to try it myself. 🙂 I think that I have some rather clear idea of how to do it and would be happy to try it out.

Thank you for all your other comments. I’ve read through all of them but would like to address them only after having explored that alternative approach of existentially quantifying at the bottom, as we might decide to follow this approach and this would make many of those comments obsolete.