Fork me on GitHub
#cljs-dev
<
2018-11-30
>
borkdude08:11:47

it seems that advanced + instrumentation is a lot faster than none + instrumentation even on Node:

Testing aos.y2017.d06.borkdude
part-1 took 1.000000 msecs
part-2 took 11597.000000 msecs
vs
Testing aos.y2017.d06.borkdude
part-1 took 7.792217 msecs
part-2 took 24212.637332 msecs
so yeah, keeping advanced instrumentation working has benefits I guess

borkdude08:11:13

(this is an advent of spec puzzle instrumented with core specs)

thheller08:11:29

@borkdude would be interesting to have the results for :simple

borkdude08:11:52

With simple:

Testing aos.y2017.d06.borkdude
part-1 took 1.932100 msecs
part-2 took 26179.181678 msecs

borkdude08:11:03

Running again with advanced:

Testing aos.y2017.d06.borkdude
part-1 took 0.000000 msecs
part-2 took 11573.000000 msecs
The last two are based on https://github.com/borkdude/clojurescript/tree/CLJS-2995 since I needed a patch to run this puzzle.

thheller08:11:30

odd. :simple should definitely not be slower than :none?

borkdude08:11:10

you can run it with: script/test-one aos.y2017.d06.borkdude and tweak the compilation setting in cljs-opts.edn

borkdude09:11:19

One more time then: Advanced:

Testing aos.y2017.d06.borkdude
part-1 took 1.000000 msecs
part-2 took 11722.000000 msecs
Simple:
Testing aos.y2017.d06.borkdude
part-1 took 1.733057 msecs
part-2 took 25785.052031 msecs
None:
Testing aos.y2017.d06.borkdude
part-1 took 2.690667 msecs
part-2 took 19662.617326 msecs
Really can’t help it 🙂

borkdude09:11:24

maybe the difference is just not that significant. but advanced does seem to help

thheller09:11:48

I'm confused but I thought calling instrument requires :static-fns false

thheller09:11:02

which defaults to true for optimized builds

borkdude09:11:54

I don’t know about that. I’m using advanced builds with speculative and all the tests that assert that an fdef should throw work

borkdude10:11:28

is there a difference in metadata on clojure.test tests with none or advanced? my tests aren’t filtered properly with advanced

mfikes12:11:52

I'm curious if 3-way merges are OK for patch application. If you take https://dev.clojure.org/jira/browse/CLJS-2920 for example,

git am ~/Downloads/CLJS-2920-2.patch
will result in "patch does not apply", but instead the patch can apply if you use --3way:
$ git am --3way ~/Downloads/CLJS-2920-2.patch
Applying: CLJS-2920: Optimize qualified- / simple- ident? / keyword? / symbol?
Using index info to reconstruct a base tree...
M	src/main/cljs/cljs/core.cljs
Falling back to patching base and 3-way merge...
Auto-merging src/main/cljs/cljs/core.cljs
(If we convince ourselves that this is OK, it could reduce some amount of re-baselining.)

tmulvaney13:11:04

Hey, @mfikes thanks for taking a look at CLJS-1888. I'm not sure if you looked at the patch or just checked that it passes. Any ideas on how best to have a cljc test or if this is even desired?

tmulvaney13:11:17

You may have noticed there is a metadata_test.cljc file which is used to confirm behaviour is correct across CLJ/CLJS but there is no real way to run this kind of thing currently, right?

thheller14:11:58

@mfikes @dnolen don't want to stir up drama but why exactly are we not using github for CLJS? I can understand the argument for CLJ when the people involved there prefer the workflow but they are not involved in CLJS anymore (AFAICT). Why not switch to something more accessible?

mfikes14:11:20

Because I’m an overpaid GitHub 🙂

mfikes14:11:30

(At least in terms of doing the ancillary crap.)

mfikes14:11:55

I would prefer GitHub FWIW.

thheller14:11:28

Might be too late because of all the issue history in Jira but the migration doesn't have to happen overnight

borkdude14:11:37

make a script

borkdude14:11:54

or no, first create a JIRA issue and then attach the script

dnolen14:11:08

@thheller yes I've argued for that, but ultimately the decision is not up to me

dnolen14:11:52

@thheller re: not true, instrument is intended to be fully advanced / fn optimization safe

dnolen14:11:52

later on, would like to circle back and consider the DCE problem

dnolen14:11:29

@mfikes I've done that before actually 3-way

dnolen14:11:24

@borkdude looks good! attach a patch to the issue please

borkdude14:11:02

mfikes tested it with CI / Canary

dnolen14:11:46

accepted some tickets w/ patches, anything else people want me to look at today?

borkdude14:11:52

maybe CLJS-2955 could get applied, because CLJS-2964 hinges on that

slipset14:11:36

FYI At work we’re using Github for source-code and Jira for issues.

borkdude15:11:00

@dnolen I mentioned CLJS-2955, but CLJS-2995 is cool too thanks 🙂

dnolen15:11:15

we should make tickets for datafy / nav protocols

dnolen15:11:45

as well as :extend-via-metadata and the required changes to make that work

mfikes15:11:15

Here is some work on datafy as it existed a while back https://dev.clojure.org/jira/browse/CLJS-2929

mfikes16:11:10

https://dev.clojure.org/jira/browse/CLJS-2968 might be nice to land on master at some point so that Canary can continue checking for GCC breaking things (right now it can’t) https://dev.clojure.org/jira/browse/CLJS-2956 seems like a straightforward blocker we could land on master

👍 4
mfikes19:11:29

OK, datafy / nav is now on master, if that helps with anything else that might depend on it being there

mfikes20:11:20

I’m taking a quick look to see if anything has progressed on the Clojure side that we should add to datafy…

slipset20:11:54

@mfikes the patch you mentioned that didn’t apply - I guess a 3-way didn’t work, so I need to rebase?

slipset20:11:37

That would be CLJS-2794 IIRC

mfikes20:11:49

ClojureScript’s datafy looks good when comparing it to Clojure’s; one remaining bit will be to eliminate a little explcit code in there when we do CLJS-2996 “Support protocol extension by metadata”. (I’ve captured this as a note in that ticket.)

mfikes20:11:28

@slipset Right, even --3way doesn’t allow that one to be applied so presumably there are some conflicts that would need manual intervention.

slipset20:11:56

Ok. I’ll fix that over the weekend sometime.