tapioca icon indicating copy to clipboard operation
tapioca copied to clipboard

`all_modules`/`all_classes` scan the object space every time

Open amomchilov opened this issue 1 year ago • 0 comments

These memoizations don't work like they were likely intended to:

https://github.com/Shopify/tapioca/blob/21a9a6dcfa9a58ccf794f09ef8a2604859dc8506/lib/tapioca/dsl/compiler.rb#L49-L64

The Enumerator is cached, but it doesn't internally cache the objects it sees when enumerated. Each call will walk the ObjectSpace over again. Minimal demo:

e = ObjectSpace.each_object(Object)
irb(main):026> e.count
38593
irb(main):027> e.count
42984 # Different count
irb(main):028> e.count
47371 # Different count

If the values were cached as intended, the counts should've been the same each time.

Proposed solution

We should probably to_a these, and cache that instead.

-   sig { returns(T::Enumerable[T::Class[T.anything]]) } 
+   sig { returns(T::Array[T::Class[T.anything]]) } 
    def all_modules
      @all_classes ||= T.let( 
-      ObjectSpace.each_object(Class), 
+      ObjectSpace.each_object(Class).to_a, 
-      T.nilable(T::Enumerable[T::Class[T.anything]]),
+      T.nilable(T::Array[T::Class[T.anything]]),
      ) 
    end 

Further ideas

We should probably also derive all_classes from all_modules. Filtering that smaller Array is faster than a second walk through the ObjectSpace.

    sig { returns(T::Array[T::Class[T.anything]]) } 
    def all_modules
      @all_classes ||= T.let( 
        all_modules.filter { Class > _1 }, 
        T.nilable(T::Array[T::Class[T.anything]]),
      ) 
    end 

amomchilov avatar Jul 30 '24 17:07 amomchilov