Should makeLenses generate code using record syntax by default? about lens HOT 6 OPEN

copying my comment on PR.

Is it faster to compile, or produces smaller Core? IIRC record updates are still compiled into Core case?

AFAIK, big records just lead to quadratic code size, if you don't go very creative, like in https://well-typed.com/blog/2021/08/large-records/

from lens.

The resulting core is slightly different. Changed the example to just two fields for brevity:

data Big = Big { _a0 :: Int , _a1 :: Int }

Then both cores for a0 start with:

a0 [InlPrag=INLINE (sat-args=2)] :: Lens' Big Int
[GblId, Arity=3, Caf=NoCafRefs, Unf=OtherCon []]
a0
  = \ (@ (x0 :: * -> *))
      ($dFunctor_a0 :: Functor x0)
      (eta_B2 :: Int -> x0 Int)
      (eta1_B1 :: Big) ->

But then the two differed, while makeLenses resumes with

      case eta1_B1 of { Big x1 x2 ->
      fmap
        @ x0
        $dFunctor_a0
        @ Int
        @ Big
        (\ (y :: Int) -> BigRecord.Big y0 x2)
        (eta_B2 x1)
      }

the new variant produces the following core:

      fmap
        @ x0
        $dFunctor_a0
        @ Int
        @ Big
        (\ (y :: Int) ->
           case eta1_B1 of { Big x1 x2 ->
           BigRecord.Big y x2
           })
        (eta_B2 (case eta1_B1 of { Big x1 x2 -> x1 }))

The difference being that the regular one matches eta1_B1 once outside and the new one matches it twice within, so it's actually longer core! So I don't know what exactly makes it less slow then..

from lens.

At a wild guess, using record syntax might save time in the type-checker because you're type-checking an expression that is constant-size rather than linear in the number of record fields? You could test this hypothesis by compiling with -ddump-timings to see where the time is being spent.

Given that the Core is larger with the new variant, I wonder if optimizing use sites might end up taking longer. And is there a semantic or runtime performance difference arising from the change?

from lens.

Given that the Core is larger with the new variant, I wonder if optimizing use sites might end up taking longer. And is there a semantic or runtime performance difference arising from the change?

For over modifier l big, over = coerce, Functor in question is Identity.

• eta1_B1 = big :: Big
• eta_B2 = modifier :: Int -> Int

-- makeLenses
   case eta1_B1 of { Big x1 x2 ->
      fmap
        @ x0
        $dFunctor_a0
        @ Int
        @ Big
        (\ (y0 :: Int) -> Big y0 x2)
        (eta_B2 x1)
      }

over modifier l big
  = case big of { Big x1 x2 ->
        (\ (y0 :: Int) -> Big y0 x2)
        (modifier x1)
      }

  =<beta-redux>
    case big of { Big x1 x2 ->
        let y0 = modifier x1
        in Big y0 x2
      }

  =<inline>
    case big of { Big x1 x2 ->
        Big (modifier x1) x2
      }

-- using records
      fmap
        @ x0
        $dFunctor_a0
        @ Int
        @ Big
        (\ (y :: Int) ->
           case eta1_B1 of { Big x1 x2 ->
           Big y x2
           })
        (eta_B2 (case eta1_B1 of { Big x1 x2 -> x1 }))

over modifier l big
  =  (\ (y :: Int) ->
           case big of { Big x1 x2 ->
           Big y x2
           })
       (modifier (case big of { Big x1 x2 -> x1 }))

  =<beta-redux>
     let y = modifier (case big of { Big x1 x2 -> x1 })
     in case big of { Big x1 x2 ->
           Big y x2
        }

  =<inline>
     case big of { Big x1 x2 ->
         Big (modifier (case big of { Big x1 x2 -> x1 })) x2
     }

  =<notice that `big` is cased on twice>
     case big of { Big x1 x2 ->
         Big (modifier x1) x2
     }

I think GHC is smart enough to figure out double case optimization. But I will be surprised if the use-site is faster to optimize.

(view case is simpler, as then the eta_B2 is replaced with coerce and fmap @(Const r) is also just coerce)

from lens.

At a wild guess, using record syntax might save time in the type-checker because you're type-checking an expression that is constant-size rather than linear in the number of record fields? You could test this hypothesis by compiling with -ddump-timings to see where the time is being spent.

Results of -ddump-timings show improvement in Renamer/typechecker over here from 2019 to 456 (btw what unit is this? milliseconds?)

Last steps:

from lens.

#987 adds an option to generate lenses using record syntax, but this option is disabled by default. I'll leave this issue open to discuss whether we should change the default.

from lens.