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…

^ @kucerm2 Have you signed the CA?

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

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

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

i could try to make it apply again

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.

Cool. I can do that.

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

Found what may be a 15% compilation perf boost 🙂

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

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

@mfikes not that ugly IMO 🙂

will take a closer look in a bit, thanks!

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

k

(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.)

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

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.

{: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}

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

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

not super relevant as huge files like that are rare

noted, worth taking a look at

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

though we should test the assumption it’s not used

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

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

possible the extra entries are to allow finer-grained breakpoints

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

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

{: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}

happens even more in other places

yeah there's no excuse for duplicates

macro-expansion maybe?

an expression from macro expansion often looks like one giant line

it makes setting breakpoints a big hassle

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

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

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

Here is the fn of interest that does that https://github.com/planck-repl/planck/blob/2014da4d470d4811f3907b35e5ae129d7e27d1a2/planck-cljs/src/planck/repl.cljs#L1388-L1402

^ 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.

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.

^ https://dev.clojure.org/jira/browse/CLJS-2895

cool thanks