brittany icon indicating copy to clipboard operation
brittany copied to clipboard

Nonoptimal layouting in complex expression with default config

Open lspitzner opened this issue 8 years ago • 1 comments

This sample code:

ugebe gdac _ekaz (NtizUbq sysn cekoed gagwwau, FdrnAqsAmnu njiuZqanacpoYbx) =
  M.egDuyz cekoed <&> \(xaldzIyaj, lfebbBopu) -> imy $ \hkhtf -> Ewzgn
    { qoahe_rb     = xaldzIyaj
    , aeroj_uxifct = Qafa.vnij "NtizUbq"
    , quzou_tgalzq = BDvkjte.dabpv
    , hivog_msnmd  = \case
      MzuhatSujaunf CurlomuVurqofOcraraw{} ->
        kmblXilIgisefu xaldzIyaj $ return []
      MzuhatSujaunf JovbiegAkujaJawqbufobd{} ->
        kmblXilIgisefu xaldzIyaj $ return []
      MzuhatSujaunf SmkojhwAkeath -> kmblXilIgisefu xaldzIyaj $ do
        pqjfyaturpI <- tubcumXzhogH
          =<< fxziqqVirephu (Qafa.vnij "vhincupedamiji")
        pure $ M.egDuyz gagwwau <&> \(iungeRyrr, lcotQjs) ->
          OJaufn iungeRyrr $ M.egDuyz lcotQjs <&> \(pAumcAweb, vdauyXuf) ->
            JItwocAfye pAumcAweb
              $   M.egDuyz vdauyXuf
              >>= \(pkesuNale, tfevaPykid) ->
                    tfevaPykid >>= \(hnaqseViof, eyubz, unzRpisupaq) ->
                      unzRpisupaq <&> \(jfhop, jva, lyi) ->
                        let
                          xmbau = cekoed M.! hnaqseViof
                          kauvx = S.tmolOobf $ case xmbau of
                            SQBokknXqazk (FUIghuGuodo _ kauvx) ->
                              [ WIwuv porp nnehajNovin []
                              | (s, e, _, _) <- kauvx
                              , let f'     = sdfmenaLodi pqjfyaturpI s
                              , let g'     = sdfmenaLodi pqjfyaturpI e
                              , let uwamm' = sdfmenaLodi pqjfyaturpI jfhop
                              , let xde'   = sdfmenaLodi pqjfyaturpI jva
                              , let nnehajNovin =
                                      yvpKwgiaQbeyxtmv $ g' `amoOB` f'
                              , uwamm' <= f'
                              , f' <= xde'
                              ]
                            UMOyumoTumoibAsvqp{} -> []
                        in
                          PYifebdruthMjob (Fsnu pkesuNale)
                                          eyubz
                                          (QEvavFipiv lyi kauvx)
    }

requires config

  lconfig_altChooser:
    tag: AltChooserBoundedSearch
    contents: 5

with the default of 3 (instead of 5) brittany reformats the above to the non-optimal:

ugebe gdac _ekaz (NtizUbq sysn cekoed gagwwau, FdrnAqsAmnu njiuZqanacpoYbx) =
  M.egDuyz cekoed <&> \(xaldzIyaj, lfebbBopu) -> imy $ \hkhtf -> Ewzgn
    { qoahe_rb     = xaldzIyaj
    , aeroj_uxifct = Qafa.vnij "NtizUbq"
    , quzou_tgalzq = BDvkjte.dabpv
    , hivog_msnmd  = \case
      MzuhatSujaunf CurlomuVurqofOcraraw{} ->
        kmblXilIgisefu xaldzIyaj $ return []
      MzuhatSujaunf JovbiegAkujaJawqbufobd{} ->
        kmblXilIgisefu xaldzIyaj $ return []
      MzuhatSujaunf SmkojhwAkeath -> kmblXilIgisefu xaldzIyaj $ do
        pqjfyaturpI <- tubcumXzhogH
          =<< fxziqqVirephu (Qafa.vnij "vhincupedamiji")
        pure $ M.egDuyz gagwwau <&> \(iungeRyrr, lcotQjs) ->
          OJaufn iungeRyrr $ M.egDuyz lcotQjs <&> \(pAumcAweb, vdauyXuf) ->
            JItwocAfye pAumcAweb
              $   M.egDuyz vdauyXuf
              >>= \(pkesuNale, tfevaPykid) ->
                    tfevaPykid
                      >>= \(hnaqseViof, eyubz, unzRpisupaq) ->
                            unzRpisupaq <&> \(jfhop, jva, lyi) ->
                              let
                                xmbau = cekoed M.! hnaqseViof
                                kauvx = S.tmolOobf $ case xmbau of
                                  SQBokknXqazk (FUIghuGuodo _ kauvx) ->
                                    [ WIwuv porp nnehajNovin []
                                    | (s, e, _, _) <- kauvx
                                    , let f' = sdfmenaLodi pqjfyaturpI s
                                    , let g' = sdfmenaLodi pqjfyaturpI e
                                    , let uwamm' =
                                            sdfmenaLodi pqjfyaturpI jfhop
                                    , let xde' = sdfmenaLodi pqjfyaturpI jva
                                    , let nnehajNovin =
                                            yvpKwgiaQbeyxtmv $ g' `amoOB` f'
                                    , uwamm' <= f'
                                    , f' <= xde'
                                    ]
                                  UMOyumoTumoibAsvqp{} -> []
                              in
                                PYifebdruthMjob (Fsnu pkesuNale)
                                                eyubz
                                                (QEvavFipiv lyi kauvx)
    }

I cannot spot any particular bug in the layouting code here and it is certainly possible that pruning to 5 instead of 3 is necessary for examples of certain complexity.

Maybe the default config should be changed? Relevant questions:

  • How big is the impact on performance for some average brittany usage when reducing the "pruning-agressiveness" from 3 to 5?
  • How often are these non-optimal cases encountered?
  • ~~Why don't we just use a list-comprehension there? It avoids tons of nesting..~~ (this only applies to this specific case; the general question is not affected really)

lspitzner avatar Jun 21 '17 15:06 lspitzner

There is another approach to this: For certain nodes, non-bottom-spacing modifier could be adapted to also apply a take 1 to the spacings. If applied carefully, this won't have any negative effects but might fix this and related issues. It'd also generally decrease the size of the search space.

lspitzner avatar Sep 11 '17 22:09 lspitzner