On 12 April 2017 at 14:17, Guillermo Polito <guillermopol...@gmail.com> wrote:
> > > On Wed, Apr 12, 2017 at 11:35 AM, Denis Kudriashov <dionisi...@gmail.com> > wrote: > >> >> 2017-04-12 10:55 GMT+02:00 Guillermo Polito <guillermopol...@gmail.com>: >> >>> PharoClassInstaller>>migrateClasses: old to: new using: >>>> anInstanceModification >>>> instanceModification := anInstanceModification. >>>> old ifEmpty: [ ^ self ]. >>>> [ >>>> 1 to: old size do: [ :index | >>>> self updateClass: (old at: index) to: (new at: index)]. >>>> old elementsForwardIdentityTo: new. >>>> " Garbage collect away the zombie instances left behind in garbage >>>> memory in #updateInstancesFrom: " >>>> " If we don't clean up this garbage, a second update would revive them >>>> with a wrong layout! " >>>> " (newClass rather than oldClass, since they are now both newClass) " >>>> Smalltalk garbageCollect. >>>> ] valueUnpreemptively >>>> >>>> Commenting garbage collection here increases performance 10 times. >>>> Then commenting class update loop increases performance 3 times more. >>>> But this loop is required. It adopts all instances of old class to new one. >>>> And time here spent in #allInstances method. >>>> >>>> Can we remove manual garbage collection here? Why it is needed? >>>> >>> >>> Well, there is the comment that explains it and makes pretty good sense. >>> >> >> But is does not explain why these bad zombies exist. We investigates >> possible reasons and could not reproduce them. We will try remove garbage >> collection here in Pharo 7 >> > > No, this will break stuff! I'll try to explain what does it mean by zombie > instances to make some sense: > > - Imagine that you have class A + 10 instances of A. > > - We add an instance variable to A. > - this means the class builder will generate class A' that is the new > version of A. > - then, it migrates all instances of A to class A'. > This migration is not magic: > - 10 new instances of A' are created > - the state is migrated from the instances of A to A' > - each instance of A is becomed into its corresponding instance of > A' > - finally we become class A into A' > This step will make that old instances of A now have: > - the old format > - but point to the new class A > > If we do not garbage collect, this means that doing > > A allInstances > > will return not only the new 10 instances of A, but the old instances of > A'. > And that will break LOOOTS of stuff. > if you run #allInstances and in between you will trigger adding instance var & GC etc etc.. you'll have everything broken.. because there are things didn't meant to work in certain scenarios. IIRC allInstances is highly dependns on NOT having full GC while doing it, and that's why all loops that doing it is highly conservative & cautious about creating new objects while iterating over heap. That's the nature how #allInstance works, and you could have a tons of issues with it regardless , if you do full GC manually, or it triggered by VM itself. So, this is nothing to do with migrating instances of class. -- Best regards, Igor Stasenko.
