kkinnear / zprint

Executables, uberjar, and library to beautifully format Clojure and Clojurescript source code and s-expressions.
MIT License
554 stars 47 forks source link

Slowness when pairing smaller width with long strings #290

Closed filip-gomore closed 1 year ago

filip-gomore commented 1 year ago

I was hoping you would be able to help us with a performance issue when we use zprint to format our Clojure code. In some files we are experiencing slowness (takes a few seconds to format the file) and I think it's connected to the string lengths because they can't get broken up. It looks hard for zprint to rule out branches and it seems to try a lot of options until it finds a solution.

Here is a prototypical example of how the code looks in those files (I just ran an obfuscation on a real function):

(defn lysnxrdilceshiq
  [{:keys [bpwwdtdrbcbeqhtzpm lbbfwa scrx as]}
   {:keys [suufsw raerqzadu] :as vjzoqll}]
  (let [zcxjsmvvgtjdjhgb (z (:rh gjnz) (:rehsrqsjammnytt fakm))
        pbaaxrgfwlf      (:rgjnpzgkigr opkf)]
    {:yniykpynbytpcl
     (isvmgl
      aezz
      [{:wmujdsnfal "ludrasz"
        :oqxmk (xi "dkualogsvwfdlodqbhpkymsosayibtdgggagmbwukjj")
        :uwdwj
        (zsnkrm
         xxjk
         [(when (vues (:yw xulg))
            (whvkjcahz
             "ctrhhbi"
             (rp "gdndmfcqbgeclpmalnjkgzdaceuoufjemuszxnqhqjusxygeblrkimafbo"
                 {:bs techhugyrzuyjqfh})
             (grtpccxmsjnm (:szpdnoueqmzoa twdk) vflrimegn hoeowpljulp)
             {:clbvjylm
              (qt
               "hwdxyisaezgwplnfljfnvyildhsjbggkbplkwjpszemjvyhawdmrfumitlowu"
               {:edofyanir (:qo udbs)
                :qvwepdygi (r (:jsyqczfxlxrzfcg ciba) (:zy ncbz))})}))])}])}))

and the .zprint.edn

{:style       [:community :justified :map-nl :pair-nl :binding-nl :respect-bl]
 :width       80
 :fn-force-nl #{:arg1-body :binding :fn}
 :map         {:comma?    false
               :sort?     true
               :force-nl? false
               :justify   {:max-variance 40}}
 :pair        {:force-nl? true :justify {:max-variance 40}}
 :binding     {:justify {:max-variance 40} :justify? true}
 :vector      {:wrap? false}
 :fn-map      {;; clojure.core and special forms
               "def"           :arg2
               "do"            :flow-body
               "try"           :flow-body
               "finally"       :flow-body}}

It takes around 1s to format the above. It's a lot faster when I change the :width to 120.

Thanks!

kkinnear commented 1 year ago

Wow, that's not good. Thank you so much for sending in "real" code. I really helps to work with the real thing.

I can reproduce this problem, and it certainly does take a long time. I have put a bunch of code in to limit how long things take when they are deep (and this is pretty deep, for sure), but the limitations I have built in don't seem to affect this particular example.

Thanks also for sending in your configuration, as I would not have known what was going on at all without it.

I have managed to isolate the time being spent to justifying maps, and in particular to doing the "smart" justifying of maps where it looks at the variance and tries to do a "good" job of handling that. I think, as you have speculated, that the problem is also related to the strings that don't fit well within the :width, as I have seen that once before where it wasn't related to justification.

Thus, if you change to this in your configuration {:map {:justify {:max-variance 0}}} (that is, change the :max-variance from 40 to 0 just for maps), it will run about 7 times faster. At least it does for me. I don't know if that will make everything look a lot worse or not, as I don't have a global view of your code base.

I have also found that you can speed it up by about a factor of two (instead of 7) by placing :map {:justify {:no-justify #{":uwdwj" ":clbvjylm"}}}. This might help, but I don't know how constant :uwdwj or the other one are. Nor if this is enough of a performance increase to be useful.

If you have code that looks like your example all over, you might need to use the :max-variance 0 approach for maps in your configuration to get the performance you need. An alternative, if there are only some places where this kind of code appears, would be use some ;!zprint directives to affect just the formatting for those functions. You could put the comment line:

;!zprint {:format :next :map {:justify {:max-variance 0}}}

immediately in front of every function that is "like" the example (and thus taking very long to format), and get back tons of performance. Like this:

;!zprint {:format :next :map {:justify {:max-variance 0}}}
(defn lysnxrdilceshiq
  [{:keys [bpwwdtdrbcbeqhtzpm lbbfwa scrx as]}
   {:keys [suufsw raerqzadu] :as vjzoqll}]
  (let [zcxjsmvvgtjdjhgb (z (:rh gjnz) (:rehsrqsjammnytt fakm))
        pbaaxrgfwlf      (:rgjnpzgkigr opkf)]
    {:yniykpynbytpcl
     (isvmgl
      aezz
      [{:wmujdsnfal "ludrasz"
        :oqxmk (xi "dkualogsvwfdlodqbhpkymsosayibtdgggagmbwukjj")
        :uwdwj
        (zsnkrm
         xxjk
         [(when (vues (:yw xulg))
            (whvkjcahz
             "ctrhhbi"
             (rp "gdndmfcqbgeclpmalnjkgzdaceuoufjemuszxnqhqjusxygeblrkimafbo"
                 {:bs techhugyrzuyjqfh})
             (grtpccxmsjnm (:szpdnoueqmzoa twdk) vflrimegn hoeowpljulp)
             {:clbvjylm
              (qt
               "hwdxyisaezgwplnfljfnvyildhsjbggkbplkwjpszemjvyhawdmrfumitlowu"
               {:edofyanir (:qo udbs)
                :qvwepdygi (r (:jsyqczfxlxrzfcg ciba) (:zy ncbz))})}))])}])}))

I'll work on figuring out why using the variance handler is taking so long, but that's not going to happen overnight and in any case it won't help you with the code that is already in the field.

I hope this helps, at least some.

filip-gomore commented 1 year ago

Thank you for taking the time to look into this and explain it so thoroughly! I can verify that setting max-variance to 0 improves performance for the whole file dramatically. We'll play around with this next week and I'm sure we'll find a solution that works for us.

filip-gomore commented 1 year ago

I'll close as we managed to improve the config for our use case (with a width set to 100 now and a few exceptions). I appreciate this tool a lot, thank you for building it!

kkinnear commented 1 year ago

I have released 1.2.6. I found several places where the justification code was missing optimizations that were available, and fixed those. Where above I saw a 7 times slowdown when doing justification using :max-variance 20, now I see a slowdown of <2. Which is a lot better, but still slower.

I also added a new configuration option: :max-depth 100 to all of the :justify key values. Part of what was an issue in your example was that there were things to justify that were pretty deeply nested. If you run into this again, you could try setting :max-depth 4 or maybe 5 or something and see if it helps. I think it will. It will only justify down to :max-depth and then it won't even try. But I hope you don't need that.

I want to thank you for the really useful example of the problem. It made a huge difference to figuring out what was wrong and correcting the slowdowns that I could correct!

Thanks for the appreciation. It means a lot!