typelevel/twiddles

Compilation hangs on Scala 2 using fairly long twiddles

francesconero opened this issue · 3 comments

Hello!

When I tried upgrading Skunk to 0.6.0, which uses this library, I discovered that compilation would hang, whereas with the previous version (using the old twiddle lists), it was fairly fast.

Digging in a bit, it seemed to be caused by some queries that selected a fair amount of columns (20 or so). I isolated a minimum example to demonstrate the problem: this snippet currently makes the compilation hang using Scala 2.13.12

import org.typelevel.twiddles._

1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
EmptyTuple

Scastie

I suspect the implicit conversion hlistToTuple in TwiddleCompat is causing unnecessary failed implicit searches exhibiting a quadratic complexity somewhere (even though the exact mechanism is not clear to me, it seems to be constantly trying to build a Tupler).

In fact, removing the implicit from scope makes it compile again.

import org.typelevel.twiddles.{hlistToTuple => _, _}

1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
1 *: 1 *: 1 *: 1 *:
EmptyTuple

Scastie

An approach I tried which seems to preserve the tuple conversion and not make the compiler hang is to define an implicit conversion for every tuple arity, mirroring the already existing tupleNtoHlist functions.

E.g.:

implicit def hlistToTuple2[Z <: Tuple, A, B](z: Z)(implicit
    tupler: Tupler.Aux[Z, (A, B)]
): (A, B) = tupler(z)

// and so on up to 22

This seems to help the compiler know the implicit is useless to build an HList. Is it because the output type is more explicit? I'm not sure.

I was planning on opening a PR to address this issue, as it is currently blocking our upgrade to the latest version of Skunk at work. We could keep using the legacy twiddles, but they would cause quite a lot of deprecation warnings. I just wanted to check which approach would be best to take first.

Thanks!

Great find! The workaround seems reasonable to me.

@francesconero I released Skunk 0.6.2 with this fix.