Clojurians
#cljs-dev
<
2018-09-06
>

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

mfikes17:09:08

I'd like to assign https://dev.clojure.org/jira/browse/CLJS-2868 to Martin Kučera to ask if the CA has been signed, but I can't find his name or ID in the assign dropdown…

mfikes17:09:25

^ @kucerm2 Have you signed the CA?

mfikes17:09:28

(Sorry to bother you @kucerm2... I couldn't find a way to ask you via JIRA about the above. :slightly_smiling_face: )

lee.justin.m17:09:23

hey @mfikes if I wanted to try to push https://dev.clojure.org/jira/browse/CLJS-1628 forward, do you think that would be worthy? it is really kind of terrible when this bug hits because it can happen when trying to print out diagnostics, which totally hides the original problem

mfikes17:09:10

Hrm. I suppose the last status was that the patch no longer applies.

lee.justin.m17:09:23

i could try to make it apply again

mfikes17:09:39

Yeah, I honestly don’t know what the protocol / etiquette is in a case like that. If it were me I’d ping Roman and see if he’s given up on it or forgot, etc.

lee.justin.m17:09:23

Cool. I can do that.

mfikes17:09:48

I suspect he’s forgotten about it… looks like it has been sitting for a year almost.

mfikes17:09:47

Found what may be a 15% compilation perf boost :slightly_smiling_face:

mfikes17:09:13

It is extremely gross, but might be worth it for this bit of tight-loop code: https://dev.clojure.org/jira/browse/CLJS-2894

john18:09:35

@dnolen & @mfikes: these performance optimizations y'all have been doing are amazing! Much kudos and appreciation!

dnolen18:09:21

@mfikes not that ugly IMO :slightly_smiling_face:

dnolen18:09:36

will take a closer look in a bit, thanks!

mfikes18:09:04

Cool. I have a follow up tweak / fix. Still testing some more and will assign to you in a bit.

mfikes18:09:08

(Hah, rough calculations show that this particular bit of code is executed billions of times in the Coal Mine test, so going for AtomicLong ends up mattering.)

dnolen18:09:01

yeah I saw the source mapping updates were in the inner loop, but I didn’t think to do anything about it

thheller19:09:15

btw the source-map data also contains a lot of data that I think might not actually be useful. I didn't check how those work yet but the source-map data is huge and writing it as cache to disk often takes longer than the actual compile.

thheller19:09:24

{:gcol 6, :gline 1473, :name "cljs.core/missing-protocol"}
                               {:gcol 33, :gline 1473}
                               {:gcol 0, :gline 1479}
                               {:gcol 0, :gline 1479}
                               {:gcol 51, :gline 1479}
                               {:gcol 0, :gline 1480}
                               {:gcol 0, :gline 1480}

thheller19:09:33

don't know how relevant all those lines without :name are but there are a lot of those

thheller19:09:12

Compile CLJS: cljs/core.cljs (3262 ms) + Cache write: cljs/core.cljs (2707 ms)

thheller19:09:45

not super relevant as huge files like that are rare

dnolen19:09:57

noted, worth taking a look at

dnolen19:09:18

I agree, no point generating source mapping info we can’t actually use

dnolen19:09:05

though we should test the assumption it’s not used

dnolen19:09:29

I remember recalling that dropping stuff even if it doesn’t have names leads to problems - was long time ago though

thheller19:09:34

quite possible. I have no idea how the encoding process works. just looked odd to me to have so many entries without :name

favila19:09:58

possible the extra entries are to allow finer-grained breakpoints

favila19:09:55

e.g. if there is one entry at each expression boundary, then could breakpoint down to the expression level even if unnamed?

thheller19:09:14

makes sense. but why would there be so many :gcol 0? like in the snippet I posted above. the same entry is repeated for 1479 and 1480

thheller19:09:35

{:gcol 55, :gline 33804, :name "cljs.core/PROTOCOL_SENTINEL"}
                                 {:gcol 0, :gline 33806}
                                 {:gcol 0, :gline 33806}
                                 {:gcol 0, :gline 33806}
                                 {:gcol 0, :gline 33811}
                                 {:gcol 0, :gline 33811}
                                 {:gcol 0, :gline 33811}
                                 {:gcol 0, :gline 33811}

thheller19:09:41

happens even more in other places

favila20:09:09

yeah there's no excuse for duplicates

favila20:09:25

macro-expansion maybe?

favila20:09:51

an expression from macro expansion often looks like one giant line

favila20:09:08

it makes setting breakpoints a big hassle

dnolen20:09:34

I don’t think macroexpansion comes in here, I think we only generate source map stuff during emission

favila20:09:10

wouldn't it depend on how the line+col metadata was copied into the new AST made by macroexpansion?

mfikes20:09:46

FWIW, Planck culls a lot of stuff out of the source maps specifically so they can be loaded quickly.

mfikes20:09:29

^ This code has been in place for about a year, with no problems. It made source map handling in Planck be instant, rather than several seconds. It would be nice if we did something similar in ClojureScript proper.

mfikes20:09:45

I can put together a candidate patch that has this in the bit of code where we write out source maps---we can assess whether to accept the patch or do something different.

dnolen20:09:48

cool thanks