rewrite-clj 2021-05-01 | Slack Archive

@lee Is there a way to do this:

(defn- rightmost?
  [zloc]
  (nil? (ws/skip z/right* ws/whitespace? (z/right* zloc))))

without relying on rewrite-clj.zip.whitespace?

borkdude15:05:54

(this is from cljstyle)

lread17:05:51

Yeah, I think so. Both skip and whitespace? from rewrite-clj.zip.whitespace are exposed to rewrite-clj.zip. So the above could be rewritten as:

(defn- rightmost?
  [zloc]
  (nil? (z/skip z/right* z/whitespace? (z/right* zloc))))

borkdude18:05:34

Yeah, I figured. Thanks :)

lread18:05:24

Rule of borkdude: if you don’t answer in three minutes, you can assume he’ll have figured it out.

ericdallo17:05:01

About the vertical align form @lee 🧵

ericdallo17:05:09

I already saw multiple preferences regarding that, people/projects/orgs that prefer to vertical align forms on everything like let/maps etcs and pople that dislike it on the other side. I think we should support for all those cases for a better UX. My preference is don't use the vertical align at all, but I agree that it works good in some places like the bb.edn tasks like @U04V15CAJ pointed in the other thread

lread17:05:41

Yeah, vertical alignment is certainly subjective.

lread17:05:11

I some cases, I find it helps my old eyeballs.

lread17:05:02

I see some chatter over in cljfmt issues about performance problems. Would you prefer a look-see into that first?

lread17:05:35

Note to lurkers: we are chatting about adding vertical alignment support to cljfmt, not rewrite-clj.

lread17:05:35

For alignment, I’ll go see what control cljstyle offers… maybe we can steal some ideas from them.

ericdallo18:05:45

Yeah, I opened this issue about performance: https://github.com/weavejester/cljfmt/issues/226 this is something that really makes users disable format on clojure-lsp because of that performance issue 😕

lread18:05:43

For alignment… might have missed it, but don’t see specific rules in cljstyle… not sure how it decides to vertically align…

lread18:05:39

For cljfmt perf… yeah I’m kind of curious. Your use case is clojure-lsp which is Clojure only, right? (not ClojureScript?).

ericdallo18:05:54

yes, there is no specification on cljstyle, I think the "common" idea is to align vertically with the longest word

👍 3

ericdallo18:05:08

Yes, clojure only, but via native image on GraalVM 👀

ericdallo18:05:36

but I noticed those performance issues on both, so I don't think graalvm is the issue

lread18:05:45

Right… maybe I’ll start by seeing if I can reproduce some slowness on the JVM. And then maybe I’ll play with com.clojure-goes-fast/clj-async-profiler to see what might be the culprits.

ericdallo18:05:53

nice, I usually notice the performance issue with medium/big forms like I mentioned on the issue. LMK if need any help/testing

ericdallo19:05:57

Found this on cljstyle: https://github.com/greglook/cljstyle/issues/47

ericdallo19:05:09

And also this 😅 https://github.com/weavejester/cljfmt/pull/77

lread20:05:36

thanks for links!

lread20:05:58

I’ll start a new thread for cljfmt perf so we don’t confuse ourselves too much!

👍 3

lread20:05:18

Related to alignment might be https://github.com/weavejester/cljfmt/issues/49 which suggests that cljfmt support an identation spec. This spec could also support vertical alignment, I suppose.

lread20:05:20

Cljfmt performance @ericdallo 🧵

lread20:05:43

I’ve made a first crack at reproducing the issue https://github.com/weavejester/cljfmt/issues/226#issuecomment-830688654

ericdallo20:05:57

Thanks! I replied it and I'm starting to think that the performance issue is not 100% deterministic, I already faced it multiple times but never found the correct repro

lread20:05:32

Criterium is careful to warm up the JVM before benchmarking, whereas with GraalVM native-image… well…

lread20:05:59

I suppose I could repeat the tests under a GraalVM native-image and see what that tells us. I’d rip out criterium for that, as I expect it does not make sense for GraalVM.

ericdallo20:05:32

I don't think it worths, I definitely saw that performance issue without using a native binary since most of the time I'm using a jar during clojure-lsp development 😅

lread20:05:54

Ah good to know.

lread20:05:03

Do the JVM times seem reasonable though? Forgetting hardware differences for a moment, if reformat took ~150ms for your provided scenario, would that be acceptable?

ericdallo20:05:12

Yes, totally, the issue when it happens, takes a lot of time, I already saw taking 25s on the clj-jondo project 😔

lread20:05:01

Woah, it would be cool if we had a reproducible case of that!

lread20:05:48

So… a culprit in sporadic slowness can be garbage collection pauses…. have you looked into that at all?

lread20:05:19

Or have you noticed any patterns at all?

ericdallo20:05:29

No, I didn't, sorry, I'll make some tests with clj-jondo project to check If I can find some kind of repro :)

lread20:05:01

Cool, if a repro would be awesome, then we can dig in!

borkdude20:05:10

What is clj-jondo?

😂 6

2021-05-01

Channels