Avoid evaluating nilCase strictly in matchList, and rename the original matchList to matchList'

[zliu41]

This shows indexing a SOP lists is slightly faster than indexing a builtin list.

[effectfully]

Ohhhhhh...

So we clearly don't need two Unit arguments, how do we get them? Well, you wrote

    go :: BI.BuiltinList a -> Integer -> a
    go xs i =
      Builtins.matchList
        xs
        (\_ -> traceError builtinListIndexTooLargeError)
        ( \hd tl _ ->
            if i `Builtins.equalsInteger` 0
              then hd
              else go tl (Builtins.subtractInteger i 1)
        )
        ()

which makes sense, you don't want to evaluate the nil case and fail at the very first element.

But why two Units? That's because matchList has a Unit inside of it to make pattern matching on lists lazy:

matchList :: forall a r . BI.BuiltinList a -> r -> (a -> BI.BuiltinList a -> r) -> r
matchList l nilCase consCase = BI.chooseList l (const nilCase) (\_ -> consCase (BI.head l) (BI.tail l)) ()

But it doesn't make the matching actually lazy! matchList always forces nilCase, so putting () inside of matchList doesn't help with laziness as that point nilCase is already forced. So you have to do the () dance again and end up with two unit arguments, which is kinda ridiculous.

I think we should have two matchList functions instead:

matchList :: forall a r . BI.BuiltinList a -> (() -> r) -> (a -> BI.BuiltinList a -> r) -> r
matchList' :: forall a r . BI.BuiltinList a -> r -> (a -> BI.BuiltinList a -> r) -> r

one for lazy matching and one for strict matching.

And while we're here we should probably have some kind of Lazy data type instead of () ->, so that we can compile it to delay and force instead of juggling the slower units.

[effectfully]

Maybe this is why we got such a dramatic reduction in MEM in your earlier PR by the way? Although I just took a cursory look and was unable to spot anything.

[zliu41]

That doesn't solve the "two unit arguments" problem, does it? You'd still end up with matchList l nilCase consCase = BI.chooseList l (const nilCase) ..., in other words BI.chooseList has to add an additional unit.

[effectfully]

No, I think it should solve the problem for the most part. You'll end up with

matchList l nilCase consCase = BI.chooseList l nilCase (const <...>) ()

Note the absence of const in the nilCase case. It'll force nilCase to WHNF, but that's a lambda and so you won't actually evaluate the result, just reach the lambda. We don't need that for consCase, since it already evaluates to a lambda, being a function.

This doesn't solve all problems, in particular if consCase is fully eta-reduced, then you'll end up evaluating both branches, but that's the usual caveat of strict languages and the answer is the usual too: don't fully eta-reduce your functions unless you actually want to evaluate the eta-reduced function.

tl;dr you'll keep the unit in the definition of matchList but won't add it to the call site of matchList.

[michaelpj]

I agree that matchList is silly and shouldn't be like that. I'm not sure we need two? We just need to delete the pointless units.

[zliu41]

You are right - I removed the redundant unit and opened #5908 for further optimization.

[michaelpj]

I was looking at this and I realised the discussion here is slightly wrong. The delays in matchList are not about the nil case, they're about the cons case. In the cons case we call head and tail; we don't want to do that unless we're definitely choosing that case (they'll fail!). So we have to delay the cons case, and that means we have to delay the nil case too.

[effectfully]

I was looking at this and I realised the discussion here is slightly wrong. The delays in matchList are not about the nil case, they're about the cons case.

The nil case is equally important, since it's an arbitrary expression and you may as well have some head in there as well. Except

In the cons case we call head and tail; we don't want to do that unless we're definitely choosing that case (they'll fail!).

in the cons case in matchList (as opposed to chooseList) we have a function, i.e. we don't call head and tail unless the function is actually called.

So it is mostly about the nil case, not the cons case in matchList. In chooseList both are equally important.

[michaelpj]

honestly, this code isn't that bad at all. well done us

[michaelpj]

why are you taking a Data argument rather than a builtin list directly?

[zliu41]

I didn't manage to lift a BuiltinList. Either the required instance is missing or I'm not doing it right.

[effectfully]

It exists:

-- See Note [Lift and Typeable instances for builtins]
instance (FromBuiltin arep a, uni `PLC.HasTermLevel` [a]) => Lift uni (BuiltinList arep) where
    lift = liftBuiltin . fromBuiltin

but it doesn't work, because the class structure is bad. The constraints say that BuiltinData must be in the universe, which it's not, hence things don't work.

[michaelpj]

I'm confused. They say the type inside the list has to be in there, but here we've got a list of integers, which should be fine?

[effectfully]

Oh, indeed, it's integers, not datas. I don't understand either then.

[zliu41]

If it's BuiltinList BuiltinData:

    • No instance for ‘plutus-core-1.25.0.0:Universe.Core.Contains
                         plutus-core-1.25.0.0:PlutusCore.Default.Universe.DefaultUni
                         PlutusTx.BuiltinData’
        arising from a use of ‘liftCodeDef’
    • In the second argument of ‘unsafeApplyCode’, namely
        ‘liftCodeDef builtinListIndexingInput’

If it's BuiltinList BuiltinInteger, then even toBuiltin doesn't work:

    • No instance for ‘PlutusTx.Builtins.Class.ToBuiltin
                         [Integer] (BI.BuiltinList Integer)’
        arising from a use of ‘PlutusTx.toBuiltin’
    • In the expression: PlutusTx.toBuiltin [1 :: Integer .. 10]

[effectfully]

If it's BuiltinList BuiltinInteger, then even toBuiltin doesn't work:

Right, that's because the instance truly doesn't exist. I've created a ticket: #5924

[michaelpj]

Surprising but interesting, I guess.