search

sxross / MotionModel

Simple Model and Validation Mixins for RubyMotion
MIT License
192 stars 67 forks source link

Fixed memory issue with NSDataDetector #100

Closed torben closed 9 years ago

torben commented 10 years ago

Hi @sxross,

After a long time using your Gem, I've just found a memory issue, when you creating a bunch of records with a string as date.

You can easily reproduce it:

Open your instruments tool with a activity monitor; Sorting after memory. Then build an application with a MotionModel named "animal". Add a field like "born_at" with :time as type. Rake and paste this into your REPL:

for i in 1..200 a = Animal.new a.born_at = "2012-12-12 12:12:12" sleep 0.3 # With the sleep command you can see the memory growing... end

Then you can see, that every model wants to have ~12 MB of the memory. The iPhone will crash after a while.

In my case I have fixed it with a class variable @detector. Maybe it is better to use a singleton? What are you thinking about that?

Thanks again for your hard work on this gem

Torben

cognitiveflux commented 9 years ago

@torben Are you planning to make the change? @sxross Is someone else already working on this?

sxross commented 9 years ago

I'm sorry -- this one completely got by me. Yes! We want to avoid excess memory consumption. The Apple docs tell us not to create a lot of data detectors because they are expensive, so at some point I was creating a class variable for that. The code was refactored into motion/date_parser.rb, which is a module included in each class. I'll have to look at your fix and find out what the merge conflicts are. It probably relates to the refactoring I mentioned. cc:/ @cognitiveflux

cognitiveflux commented 9 years ago

Would moving @detector to a class variable @@detector just like @@isoDateFormatter suffice (updating the methods appropriately and similar to self.allocate_date_formatter)?

sxross commented 9 years ago

I'm sorry not to have fixed this as a pull, @torben, but there was a merge conflict, so I just typed in the fix myself. If you could verify this in Instruments yourself, I'd really appreciate it.