clojure-dev

dmiller 2023-01-31T17:23:40.675029Z

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) 2023-02-01T15:24:12.072439Z

> 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 ;)

dmiller 2023-02-01T15:25:00.554859Z

But why only in doEquiv and not in doEquals?

dmiller 2023-02-01T15:25:55.740939Z

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

Alex Miller (Clojure team) 2023-02-01T15:26:44.327699Z

off the top of my head, don't know

dmiller 2023-02-01T15:28:45.294859Z

That was a fairly recent commit: https://github.com/clojure/clojure/commit/b30a99014814f2fe68833622a567c80133b0c417

Alex Miller (Clojure team) 2023-02-01T15:36:36.307589Z

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

Alex Miller (Clojure team) 2023-02-01T15:39:09.738489Z

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

dmiller 2023-02-01T15:43:06.534279Z

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

Alex Miller (Clojure team) 2023-02-01T15:56:45.464989Z

seems likely

Alex Miller (Clojure team) 2023-02-01T17:06:12.375399Z

I created a jira for that https://clojure.atlassian.net/browse/CLJ-2746

dmiller 2023-02-01T17:11:28.578549Z

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