Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nonoptimal layouting in complex expression with default config #33

Open
lspitzner opened this issue Jun 21, 2017 · 1 comment
Open

Nonoptimal layouting in complex expression with default config #33

lspitzner opened this issue Jun 21, 2017 · 1 comment

Comments

@lspitzner
Copy link
Owner

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
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants