So what's the rationale for handling instances of compact classes
specially?  Conceptually compact instances _do_ point to their classes;
they just have a funky encoding of that reference.  Further, that is
something that is conceptually local to the VM.  It only leaks out because
the image is allowed to/required to manage the compact classes array.  That
e.g. markAndTrace in the VM doesn't indirect through compact classes is an
optimization local to the VM.  The GC could be written to trace the classes
of compact instances.  Since it isn't strict;y necessary the VM avoids the
cost.  But st least conceptually a compact class reference is just a
space-efficient encoding of a class reference.

Personally I think this is a mistake.  If the VM's implementation changes
(e.g. as per my two word object header proposal then all class references
in objects become indirect) then the below code will have to be backed out.
 Seems to me like we're allowing optimizations to leak up out of the VM to
unnecessarily pollute and complicate the object model.

On Sun, Jan 8, 2012 at 5:47 PM, <commits at source.squeak.org> wrote:

> A new version of Kernel was added to project The Inbox:
> http://source.squeak.org/inbox/Kernel-ul.664.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-ul.664
> Author: ul
> Time: 9 January 2012, 2:46:30.214 am
> UUID: cc70b7c6-5bbd-4a4e-9bd1-4a22915a61de
> Ancestors: Kernel-bf.663
>
> Enhanced pointer tracing:
> - handle SmallIntegers correctly (they don't point to any object)
> - handle instances of compact classes correctly (they don't point to their
> class)
> - weak references are ignored, because they don't stop the garbage
> collector in collecting objects
> These changes modify the behavior of #pointsTo:, #outboundPointersDo: and
> #inboundPointersExcluding:.
>
> =============== Diff against Kernel-bf.663 ===============
>
> Item was added:
> + ----- Method: Behavior>>isCompact (in category 'testing') -----
> + isCompact
> +
> +       ^self indexIfCompact ~= 0!
>
> Item was changed:
>  ----- Method: CompiledMethod>>outboundPointersDo: (in category 'tracing')
> -----
>  outboundPointersDo: aBlock
> +       "Evaluate aBlock for all objects I am causing not to be
> garbage-collected."
>
> +       self class isCompact ifFalse: [ aBlock value: self class ].
> +       1 to: self numLiterals do: [ :i | aBlock value: (self literalAt:
> i) ]!
> -       | numLiterals |
> -       aBlock value: self class.
> -       numLiterals := self numLiterals.
> -       1 to: numLiterals do: [:i | aBlock value: (self literalAt: i)]!
>
> Item was changed:
>  ----- Method: Object>>inboundPointers (in category 'tracing') -----
>  inboundPointers
> +       "Answer a list of all objects in the system that hold a strong
> reference to me."
> - "Answers a collection of all objects in the system that point to myself"
>
>        ^ self inboundPointersExcluding: #()!
>
> Item was changed:
>  ----- Method: Object>>inboundPointersExcluding: (in category 'tracing')
> -----
>  inboundPointersExcluding: objectsToExclude
> +       "Answer a list of all objects in the system that hold a strong
> reference to me, excluding those in the collection of objectsToExclude."
> - "Answer a list of all objects in the system that point to me, excluding
> those in the collection of objectsToExclude. I do my best to avoid creating
> any temporary objects that point to myself, especially method and block
> contexts. Adapted from PointerFinder class >> #pointersTo:except:"
>
> +       | pointers object objectsToAlwaysExclude |
> -       | anObj pointers objectsToAlwaysExclude |
>        Smalltalk garbageCollect.
> +       pointers := OrderedCollection new.
> +       "SystemNavigation >> #allObjectsDo: is inlined here with a slight
> modification: the marker object is pointers. This gives better results,
> because the value of pointers, it's inner objects and transient method
> contexts will not be iterated over."
> +       object := self someObject.
> +       [ object == pointers ] whileFalse: [
> +               (object isInMemory and: [ object pointsTo: self ]) ifTrue:
> [
> +                       pointers add: object ].
> +               object := object nextObject ].
> -       "big collection shouldn't grow, so it's contents array is always
> the same"
> -       pointers := OrderedCollection new: 1000.
> -
> -       "#allObjectsDo: and #pointsTo: are expanded inline to keep spurious
> -        method and block contexts out of the results"
> -       anObj := self someObject.
> -       [0 == anObj] whileFalse: [ "We must use #== here, to avoid leaving
> the loop when anObj is another number that's equal to 0 (e.g. 0.0)."
> -               anObj isInMemory
> -                       ifTrue: [((anObj instVarsInclude: self)
> -                               or: [anObj class == self])
> -                                       ifTrue: [pointers add: anObj]].
> -               anObj := anObj nextObject].
> -
>        objectsToAlwaysExclude := {
> -               pointers collector.
>                thisContext.
>                thisContext sender.
>                thisContext sender sender.
>                objectsToExclude.
>        }.
> +       ^pointers removeAllSuchThat: [ :ea |
> -
> -       ^ pointers removeAllSuchThat: [:ea |
>                (objectsToAlwaysExclude identityIncludes: ea)
> +                       or: [objectsToExclude identityIncludes: ea ] ]!
> -                       or: [objectsToExclude identityIncludes: ea]]!
>
> Item was changed:
>  ----- Method: Object>>outboundPointers (in category 'tracing') -----
>  outboundPointers
> +       "Answers a list of all objects I am causing not to be garbage
> collected"
> - "Answers a list of all objects I am causing not to be garbage-collected"
>
>        | collection |
>        collection := OrderedCollection new.
>        self outboundPointersDo: [:ea | collection add: ea].
>        ^ collection!
>
> Item was changed:
>  ----- Method: Object>>outboundPointersDo: (in category 'tracing') -----
>  outboundPointersDo: aBlock
> +       "Evaluate aBlock for all objects I am causing not to be
> garbage-collected."
> - "do aBlock for every object I point to, exactly how the garbage
> collector would. Adapted from PointerFinder >> #followObject:"
>
> +       self class isCompact ifFalse: [ aBlock value: self class ].
> -       aBlock value: self class.
>        1 to: self class instSize do: [:i | aBlock value: (self instVarAt:
> i)].
> +       self class isWeak ifFalse: [
> +               1 to: self basicSize do: [:i | aBlock value: (self
> basicAt: i)] ]!
> -       1 to: self basicSize do: [:i | aBlock value: (self basicAt: i)].!
>
> Item was changed:
>  ----- Method: ProtoObject>>pointsTo: (in category 'tracing') -----
>  pointsTo: anObject
> +       "Answers true if the garbage collector would fail to collect
> anObject because I hold a reference to it, or false otherwise"
> - "Answers true if I hold a reference to anObject, or false otherwise. Or
> stated another way:
>
> +       (self instVarsInclude: anObject)
> +               ifTrue: [
> +                       self class isWeak ifFalse: [ ^true ].
> +                       1 to: self class instSize do: [ :i |
> +                               (self instVarAt: i) == anObject ifTrue: [
> ^true ] ].
> +                       ^false ]
> +               ifFalse: [ ^self class == anObject and: [ self class
> isCompact not ] ]!
> - Answers true if the garbage collector would fail to collect anObject
> because I hold a reference to it, or false otherwise"
> -
> -       ^ (self instVarsInclude: anObject)
> -               or: [self class == anObject]!
>
> Item was added:
> + ----- Method: SmallInteger>>outboundPointersDo: (in category 'tracing')
> -----
> + outboundPointersDo: aBlock
> +       "Evaluate aBlock for all objects I am causing not to be
> garbage-collected."!
>
> Item was added:
> + ----- Method: SmallInteger>>pointsTo: (in category 'tracing') -----
> + pointsTo: anObject
> +       "Answers true if the garbage collector would fail to collect
> anObject because I hold a reference to it, or false otherwise"
> +
> +       ^false!
>
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20120109/4eb1c5b0/attachment.htm