This was originally filed with respect to transient collections, which are countable, but not seqable, but I’ve widened it as it’s really an issue with any non-seqable collection.

There are good reasons for transient collections not to be Seqable - seqs imply caching, caching hurts perf, and the whole reason to be using transients is for batch load perf. Similarly, iterators are stateful and transients are mutable so that does not seem like a good path for transients. But it is quite reasonable to want to know whether a transient you are building has anything in it yet via empty?.

Separately, needing to convert a Counted collection to a seq to check emptiness is usually slower than just checking the count.

Original example that drove this ticket (obviously the check would probably be used in the midst of an algorithm, not in this way, this is just a small example):

A workaround is to use (zero? (count (transient ...))) check instead.

Cause: empty? is based on seqability, which transients don't implement.

Proposed Add a branch to empty? for counted? colls. Transients implement Counted so gain support via this branch. Other colls that are counted are also faster. Seq branch continues to work for seqable non-Counted colls as before.

Perf test:

Results:

Not sure if doc string should be tweaked to be more generic, particularly the "same as (not (seq coll))" which is now only true for Seqable but not Counted. I think the advise to use (seq coll) for seq checks is still good there.

I did a skim for other types that are Counted but not seqs/Seqable and didn't find much other than internal things like ChunkBuffer. Many are both and would thus use the counted path instead (all the persistent colls for example and any kind of IndexedSeq).

I guess another option would be just to fully switch empty? to be about (zero? (bounded-count 1 coll)) and lean on count's polymorphism completely.

Patch: clj-1872-2.patch

Screened by: fogus