Model.all should return a new collection each time.

[vaughankg]

Seems like x should be unchanged in the following example. What do you think?

x = Model.all #=> []
Model.create
x #=>[<Model#1:0x111ea5d0>]

Using the ArrayModelAdapter

[sxross]

This change makes me nervous. Using dup on a collection will, at a minimum, create a lot of garbage each time the all method is called. At worst, instances of a previously returned all may remain bound to variables in scope and not garbage collected. Would you be able to instrument this? I'd be happy to be proven wrong.

[vaughankg]

Cool I'll look into it.

[vaughankg]

Hey, I'm not sure exactly what you are after with regards to the garbage collection. Can you sketch it out for me?

I've been exploring the code and these are a couple of observations:

Calling all returns direct access to the models @collection variable. Probably a bad idea because you really shouldn't be able to do the following:

x = Task.all
x.push("eggs")
Task.all #=> ["eggs"]

I also think that Task.all and Task.where{ true }.all should behave similarly.

Currently ArrayFinderQuery will (mostly) return a new collection object by way of calling the collect method.

Task.all.object_id #=> 156194608
Task.where(:name).all.object_id #=> 156194608
Task.where(:name).contains('').all.object_id #=>155272720   <- different
Task.where{ true }.all.object_id #=> 152138352 <- different

[sxross]

I'm not disputing your evaluation of the semantics of what all means. What concerns me is that by duping the whole collection, you create a ton of objects in a constrained memory environment. Additionally, dup is a shallow copy, so you might have unpredictable results if you rely on it to do a deep copy. Finally, ARC relies on reference counting and if there is a variable in the client application that is tied to the original collection, that collection would still be bound into memory and not harvested.

Again, I'm not totally certain of this, but it seems unsafe to make this kind of change. Let's keep this discussion open and you think about it. It's an efficiency (memory/speed) versus semantic difference and in this case, we discovered that the efficiency made sense to pursue.

If you want to check your branch, create an app that does what you propose using dup and run it under Instruments. Watch and see if ARC is really managing the memory over time.

Thanks!

[vaughankg]

Sounds good. I'll play with my dup branch and report any results.