Fork me on GitHub
#clojure-dev
<
2023-01-31
>
dmiller17:01:40

I'm looking at the code for APersistentVector and there is a piece of code that I am having trouble understanding. I'm comparing doEquals with doEquiv. I would expect them to be quite similar, the primary difference being that s as you move through the vector and thing you're comparing against one will use Util.equal to compare items and the other will use Util.equiv. Each has three cases: (1) comparing against an IPersistentVector; (2) comparing against a java.util.List; (3) otherwise. For cases (1) and (3), indeed they are identical except for Util.equal vs Util.equiv. But for case (2), there is a difference. In doEquals, there is a short-circuit test to get us out early:

Collection ma = (Collection) obj;     // obj is what are comparing to, v is IPersistentVector 		
if(ma.size() != v.count() || ma.hashCode() != v.hashCode())
    return false;
In doEquiv, the short-circuit code is
Collection ma = (Collection) obj; 		
if((!(ma instanceof IPersistentCollection) || (ma instanceof Counted)) && (ma.size() != v.count()))
    return false;
I can understand the hashcode check going away -- more things are 'equiv' than 'equals' and the hashcode consistency will only work for Equals. I read the guard in the doEquiv code as trying to avoid calling .size() on an IPersistentCollection that is not Counted so that we avoid a potentially costly computation. My question is: why is that guard not on the doEquals case? What am I missing? (Or is it just missing?)

Alex Miller (Clojure team)15:02:12

> trying to avoid calling .size() on an IPersistentCollection that is not Counted so that we avoid a potentially costly computation definitely, yes (specifically infinite seqs, which are very expensive to count ;)

dmiller15:02:00

But why only in doEquiv and not in doEquals?

dmiller15:02:55

(My training was in logic and set theory. We counted the uncountable before lunch.)

Alex Miller (Clojure team)15:02:44

off the top of my head, don't know

Alex Miller (Clojure team)15:02:36

oh that rings a bell now. maybe doEquals should have changed similarly

Alex Miller (Clojure team)15:02:09

I'll ask Fogus since he worked on that to see if he remembers more

dmiller15:02:06

Now that I look at the commit, same would be true for ASeq.equiv vs ASeq.Equals.

dmiller17:02:28

Cool. Always nice to find out I wasn't just being thick. 🙂 Thanks.