For contrib libraries, where does an unsolicited (no pre-existing jira ticket asking for it) performance improvement patch go? First create the http://ask.clojure.org -> contrib owner creates the jira -> attach the patch? Or would non-owner contributors open a Jira directly and attach the patch?
you would create an http://ask.clojure.org, then someone will decide if it is something that actually needs addressing
then a jira issue would be created, and patches if and when they exist would be attached there
you need to have signed a contributor agreement before you can submit patches (or get a jira account)
I have both signed the CLA and a jira account now
depending it might be a very up hill climb
my experience is rich really doesn't like it when issues coming with a patch attached
is rich still hands-on with the contrib libraries?
@joshlemer Very much depends on the Contrib lib maintainer -- which lib(s) are you thinking about?
data.int-map
@alexmiller has mostly been taking care of that recently so he can probably provide more specific guidance on how he'd like work on that approached...?
hey @joshlemer for contrib, if you're working tightly with a project, it's ok in this case for you to create the jira directly
just as an fyi, Fogus and I try to cycle through and look at various contrib projects on Fridays so if you submit some stuff and don't see activity for a bit, that's normal. but feel free to ping me here if you're looking for feedback on something or if it's been a while etc
Thanks Alex!
I think get-in could short circuit (for performance) if the intermediate lookup thingie is nil - has this ever been considered?
Jmh running Hopefully I configured it somewhat correctly
Let's start with using reduce over reduce1:upside_down_face:
don't remember it being considered
;; original get-in: 357ms
(time (dotimes [_ 10000000]
(get-in {:a {}} [:a :b :c :d :e])))
;; get-in + reduce1 -> reduce: 265ms
(time (dotimes [_ 10000000]
(get-in-old {:a {}} [:a :b :c :d :e])))
;; get-in + reduce + short circuit on nil: 134ms
(time (dotimes [_ 10000000]
(get-in-new {:a {}} [:a :b :c :d :e])))Let me know if there's interest in an issue.
;; 94ms
(time (dotimes [_ 10000000]
(some-> {:a {}} :a :b :c :d :e)))For comparison, can you check it against non-empty maps? Also, what about the not-found case?
non-empty map with get-in + reduce:
;; 300 ms
(time (dotimes [_ 10000000]
(get-in-old {:a {:b {:c {:d {:e 1}}}}} [:a :b :c :d :e])))non-empty map with get-in + reduce + short circuit:
;; 224ms
(time (dotimes [_ 10000000]
(get-in-new {:a {:b {:c {:d {:e 1}}}}} [:a :b :c :d :e])))I can't explain why the short-circuit version seems to be faster with a non-empty map, but I get it consistently
are you comparing it to the version which uses reduce1 ?
no, comparing to get-in + reduce like I wrote
🤔
Here's my messy file, feel free to play...
I haven't changed the not-found case, but similar things can be done there
1 2 you know what to do
I'll make an "ask" tomorrow, thanks ;)
Could the short circuit case be faster because the compiler can prove get doesn't get called on nil
I hope I'll have time, I can try testing this with jmh
Or is the concern that someone could extend nil to ILookup (imo not a good thing to do, but ...)
Say that for JVM.
Yeah, but even then, the clojure implementation could keep track of nil being extended to ILookup and invalidate that assumption. It's bad practice anyway to extend types you don't own, so one could also say: you're not supposed to do that
that's such a horrible devilish idea, i am in awe
ILookup is an interface, you can't "extend" it to nil
ah nice
If I'm not mistaken, I think that in java an inner (non-static) class will retain a reference to its outer class instance, and stop it from being GC'd, even if the class never actually accesses anything from the enclosing instance, is that right? Namely, IntSet.BitSetContainer and IntSet.SingleContainer are inner, non-static classes of IntSet, even though they can/do outlive their original IntSet, and don't actually access their enclosing instance. So it could be the case that they end up blocking GC of whatever IntSet they originated from. https://github.com/clojure/data.int-map/blob/master/src/main/java/clojure/data/int_map/IntSet.java#L18
(as well as taking up extra memory by storing a reference to the enclosing instance)
if there's no path back to a gc root, the whole thing will get gc'ed, so maybe not a big deal, but certainly if the inner class does not use the outer class field state, it should just be static
JVM garbage collectors CAN collect dead data even with cycles of references, unlike reference counting garbage collectors.
Not talking about dead data here, rather an instance of the inner class can outlive its outer instance, holding onto its reference
IE the child is still reachable