Bug #16026

`Set#count` performance issues

Added by ioquatix (Samuel Williams) 6 months ago. Updated 5 months ago.

Target version:


Set#size is O(1), but Set#count is O(N).

I would like to add alias count size to class Set

Is it okay?


Updated by Hanmac (Hans Mackowiak) 6 months ago

technically Set#count comes from Enumerable, where it has support for this too

enum.count(item)           -> int
enum.count { |obj| block } -> int

so i don't think Set should make #count work like #size

Updated by ioquatix (Samuel Williams) 6 months ago

I understand and I agree with your logic.

Maybe it's worth considering, #count with no arguments can invoke #size. What do you think?

Updated by byroot (Jean Boussier) 6 months ago

Maybe it's worth considering, #count with no arguments can invoke #size. What do you think?

Since it's what Array does (rb_ary_count), it would make sense to do the same in set.rb IMHO.

Updated by ioquatix (Samuel Williams) 5 months ago

I was okay with changing my implementation to use #size - which is what I've done. However, I don't think there is anything wrong with optimising this use case if it doesn't add any overhead to existing use case. Because Array does it too, it makes me think it's not a completely stupid idea.

If that's the case, maybe we should consider whether this logic belongs in Enumerable.

e.g. Enumerable#count -> Enumerable#size, and Enumerable#size is implemented by calling #each by default. However, maybe that introduce bad behaviour/expectations about behaviour. Not sure. Enumerable#size makes less sense to me, but it would be consistent across how it's used if we implemented as above.

Updated by knu (Akinori MUSHA) 5 months ago

I agree that the return size if !block_given? && respond_to?(:size) logic should belong to Enumerable. (oh, and also if item is not given)

Updated by Hanmac (Hans Mackowiak) 5 months ago

it might not be able to be generic in Enumerable

for example an Widget might be Enumerable with the child widgets, but its size would be [width, height]

for Set#count

class Set
  def count(*args)
    return size if !block_given? && args.empty?

Updated by ioquatix (Samuel Williams) 5 months ago

  • Status changed from Open to Rejected

Given the discussion, I'm okay if we just reject this feature. knu (Akinori MUSHA) it's up to you if you want to implement it or not. Even thought it can make sense from one POV, the correct solution for most users is just to use #size when it's available, rather than making #count more complicated implementation (and potentially slower).

Ultimately, it was my fault for not understanding #count.

Also available in: Atom PDF