The Inbox: Kernel-ul.664.mcz
Locked
 
|
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! |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On 09.01.2012, at 01:47, [hidden email] 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:. The pointsTo: method used to be the fastest way to test for inclusion in performance-sensitive code. Maybe instead of changing its behavior, make a new method pointsStronglyTo:? I'd revert pointsTo: back to its original which goes straight to the primitive. PointerFinder and Co. could use the new, more exhaustive, but ultimately slower method. The proposed method is still 5x faster than unoptimized code, but the original was more than 10x faster. n := 10000000. { 'includes:' -> [1 to: n do: [:i | #(up down left right) includes: #right] ] timeToRun. 'identityIncludes:' -> [1 to: n do: [:i | #(up down left right) identityIncludes: #right] ] timeToRun. 'proposed pointsTo:' -> [1 to: n do: [:i | #(up down left right) pointsStronglyTo: #right] ] timeToRun. 'current pointsTo:' -> [1 to: n do: [:i | #(up down left right) pointsTo: #right] ] timeToRun. 'original pointsTo:' -> [1 to: n do: [:i | #(up down left right) instVarsInclude: #right] ] timeToRun. } ==> { 'includes:'->1668 . 'identityIncludes:'->1925 . 'proposed pointsTo:'->314 . 'current pointsTo:'->178 . 'original pointsTo:'->131 } - Bert - For reference, this is primitive 132: primitiveObjectPointsTo | rcvr thang lastField | thang := self popStack. rcvr := self popStack. (self isIntegerObject: rcvr) ifTrue: [^self pushBool: false]. lastField := self lastPointerOf: rcvr. self baseHeaderSize to: lastField by: self bytesPerWord do: [:i | (self longAt: rcvr + i) = thang ifTrue: [^ self pushBool: true]]. self pushBool: false. |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On Mon, 9 Jan 2012, Bert Freudenberg wrote:
> On 09.01.2012, at 01:47, [hidden email] 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:. > > > The pointsTo: method used to be the fastest way to test for inclusion in performance-sensitive code. Maybe instead of changing its behavior, make a new method pointsStronglyTo:? I'd revert pointsTo: back to its original which goes straight to the primitive. PointerFinder and Co. could use the new, more exhaustive, but ultimately slower method. The proposed method is still 5x faster than unoptimized code, but the original was more than 10x faster. #pointsTo:'s behavior was already changed in 4.3, because the primitive doesn't check the class pointer (I think this change is ok). The fast method with primitive 132 is called #instVarsInclude: in 4.3. I'm not sure if it's a good idea to revert #pointsTo:. I proposed a new primitive to Eliot which would be more useful and could replace primitive 132 without significant loss in performance in long term. Most of these changes are just fixes to pointer tracing. The major change (which is the reason why I pushed it to the Inbox) is that weak references will ignored by these methods. I wonder if anyone would miss them. If yes, then we can create new variants of these methods with an additional boolean parameter which defines the behavior of weak references, e.g. #pointsTo:includingWeakReferences:. The default could either be true (to preserve the original behavior) or false (which is what we really need in practice IMHO). > > n := 10000000. > { > 'includes:' -> [1 to: n do: [:i | > #(up down left right) includes: #right] > ] timeToRun. > 'identityIncludes:' -> [1 to: n do: [:i | > #(up down left right) identityIncludes: #right] > ] timeToRun. > 'proposed pointsTo:' -> [1 to: n do: [:i | > #(up down left right) pointsStronglyTo: #right] > ] timeToRun. > 'current pointsTo:' -> [1 to: n do: [:i | > #(up down left right) pointsTo: #right] > ] timeToRun. > 'original pointsTo:' -> [1 to: n do: [:i | > #(up down left right) instVarsInclude: #right] > ] timeToRun. > } > > ==> { 'includes:'->1668 . > 'identityIncludes:'->1925 . > 'proposed pointsTo:'->314 . > 'current pointsTo:'->178 . > 'original pointsTo:'->131 } > > - Bert - > > For reference, this is primitive 132: My proposal for the new primitive was to return the index instead of true and 0 instead of false. This change is very simple in the interpreter, but it's a lot more complicated in Cog. Levente > > primitiveObjectPointsTo > | rcvr thang lastField | > thang := self popStack. > rcvr := self popStack. > (self isIntegerObject: rcvr) ifTrue: [^self pushBool: false]. > > lastField := self lastPointerOf: rcvr. > self baseHeaderSize to: lastField by: self bytesPerWord do: > [:i | (self longAt: rcvr + i) = thang > ifTrue: [^ self pushBool: true]]. > self pushBool: false. > > > |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On 09.01.2012, at 12:24, Levente Uzonyi wrote: > On Mon, 9 Jan 2012, Bert Freudenberg wrote: > >> On 09.01.2012, at 01:47, [hidden email] 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:. >> >> >> The pointsTo: method used to be the fastest way to test for inclusion in performance-sensitive code. Maybe instead of changing its behavior, make a new method pointsStronglyTo:? I'd revert pointsTo: back to its original which goes straight to the primitive. PointerFinder and Co. could use the new, more exhaustive, but ultimately slower method. The proposed method is still 5x faster than unoptimized code, but the original was more than 10x faster. > > #pointsTo:'s behavior was already changed in 4.3, because the primitive doesn't check the class pointer (I think this change is ok). The fast method with primitive 132 is called #instVarsInclude: in 4.3. I know, see my example code. I did not load your code but just added your proposed pointsTo: as pointsStronglyTo:, to be able to compare. I don't like the name "instVarsInclude:" because it is incorrect. For one, it actually applies to all slots in the receiver, both inst vars and indexed fields. And secondly, it's looking for identity. So it's more like "instVarsOrIndexedFieldsIdentityInclude:" (not that I would propose that selector). This slipped by me when Matthew added it. > I'm not sure if it's a good idea to revert #pointsTo:. I proposed a new primitive to Eliot which would be more useful and could replace primitive 132 without significant loss in performance in long term. What would the new primitive's behavior be regarding weak refs? And would it answer -1 for the class pointer? > Most of these changes are just fixes to pointer tracing. The major change (which is the reason why I pushed it to the Inbox) is that weak references will ignored by these methods. I wonder if anyone would miss them. I wouldn't miss them, I'm just concerned with speed. > If yes, then we can create new variants of these methods with an additional boolean parameter which defines the behavior of weak references, e.g. #pointsTo:includingWeakReferences:. The default could either be true (to preserve the original behavior) or false (which is what we really need in practice IMHO). We need something that matches exactly what the garbage collector does, agreed. The question is, is that enough, or is a super-fast "any field points to" method (using primitive 132 directly) still useful, and if so, what should its name be. >> n := 10000000. >> { >> 'includes:' -> [1 to: n do: [:i | >> #(up down left right) includes: #right] >> ] timeToRun. >> 'identityIncludes:' -> [1 to: n do: [:i | >> #(up down left right) identityIncludes: #right] >> ] timeToRun. >> 'proposed pointsTo:' -> [1 to: n do: [:i | >> #(up down left right) pointsStronglyTo: #right] >> ] timeToRun. >> 'current pointsTo:' -> [1 to: n do: [:i | >> #(up down left right) pointsTo: #right] >> ] timeToRun. >> 'original pointsTo:' -> [1 to: n do: [:i | >> #(up down left right) instVarsInclude: #right] >> ] timeToRun. >> } >> >> ==> { 'includes:'->1668 . >> 'identityIncludes:'->1925 . >> 'proposed pointsTo:'->314 . >> 'current pointsTo:'->178 . >> 'original pointsTo:'->131 } >> >> - Bert - >> >> For reference, this is primitive 132: > > My proposal for the new primitive was to return the index instead of true and 0 instead of false. This change is very simple in the interpreter, but it's a lot more complicated in Cog. > > > Levente Interesting. I thought the object memory was pretty much identical under Cog, except for Float byte order? Why is it more complicated? - Bert - >> >> primitiveObjectPointsTo >> | rcvr thang lastField | >> thang := self popStack. >> rcvr := self popStack. >> (self isIntegerObject: rcvr) ifTrue: [^self pushBool: false]. >> >> lastField := self lastPointerOf: rcvr. >> self baseHeaderSize to: lastField by: self bytesPerWord do: >> [:i | (self longAt: rcvr + i) = thang >> ifTrue: [^ self pushBool: true]]. >> self pushBool: false. >> >> >> > |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On Mon, 9 Jan 2012, Bert Freudenberg wrote:
> > On 09.01.2012, at 12:24, Levente Uzonyi wrote: > >> On Mon, 9 Jan 2012, Bert Freudenberg wrote: >> >>> On 09.01.2012, at 01:47, [hidden email] 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:. >>> >>> >>> The pointsTo: method used to be the fastest way to test for inclusion in performance-sensitive code. Maybe instead of changing its behavior, make a new method pointsStronglyTo:? I'd revert pointsTo: back to its original which goes straight to the primitive. PointerFinder and Co. could use the new, more exhaustive, but ultimately slower method. The proposed method is still 5x faster than unoptimized code, but the original was more than 10x faster. >> >> #pointsTo:'s behavior was already changed in 4.3, because the primitive doesn't check the class pointer (I think this change is ok). The fast method with primitive 132 is called #instVarsInclude: in 4.3. > > I know, see my example code. I did not load your code but just added your proposed pointsTo: as pointsStronglyTo:, to be able to compare. > > I don't like the name "instVarsInclude:" because it is incorrect. For one, it actually applies to all slots in the receiver, both inst vars and indexed fields. And secondly, it's looking for identity. So it's more like "instVarsOrIndexedFieldsIdentityInclude:" (not that I would propose that selector). This slipped by me when Matthew added it. I don't like the name either. > >> I'm not sure if it's a good idea to revert #pointsTo:. I proposed a new primitive to Eliot which would be more useful and could replace primitive 132 without significant loss in performance in long term. > > What would the new primitive's behavior be regarding weak refs? And would it answer -1 for the class pointer? The primitive would behave just like 132, the only difference is the return value. So it would return the index of weak refs and wouldn't check the class pointer. My goal is to have a primitive that can be used for fast linear search based on identity. It would be even better to add a startIndex argument to it. With this primitive we could have faster and simpler large identity-based hash tables for example. Adding the class pointer check to the VM is a good idea, using -1 as the return value sounds ok to me if the class pointer is checked after the slots. > >> Most of these changes are just fixes to pointer tracing. The major change (which is the reason why I pushed it to the Inbox) is that weak references will ignored by these methods. I wonder if anyone would miss them. > > I wouldn't miss them, I'm just concerned with speed. How important is the performance of #pointsTo:? > >> If yes, then we can create new variants of these methods with an additional boolean parameter which defines the behavior of weak references, e.g. #pointsTo:includingWeakReferences:. The default could either be true (to preserve the original behavior) or false (which is what we really need in practice IMHO). > > We need something that matches exactly what the garbage collector does, agreed. The question is, is that enough, or is a super-fast "any field points to" method (using primitive 132 directly) still useful, and if so, what should its name be. I'm not against the latter. > >>> n := 10000000. >>> { >>> 'includes:' -> [1 to: n do: [:i | >>> #(up down left right) includes: #right] >>> ] timeToRun. >>> 'identityIncludes:' -> [1 to: n do: [:i | >>> #(up down left right) identityIncludes: #right] >>> ] timeToRun. >>> 'proposed pointsTo:' -> [1 to: n do: [:i | >>> #(up down left right) pointsStronglyTo: #right] >>> ] timeToRun. >>> 'current pointsTo:' -> [1 to: n do: [:i | >>> #(up down left right) pointsTo: #right] >>> ] timeToRun. >>> 'original pointsTo:' -> [1 to: n do: [:i | >>> #(up down left right) instVarsInclude: #right] >>> ] timeToRun. >>> } >>> >>> ==> { 'includes:'->1668 . >>> 'identityIncludes:'->1925 . >>> 'proposed pointsTo:'->314 . >>> 'current pointsTo:'->178 . >>> 'original pointsTo:'->131 } >>> >>> - Bert - >>> >>> For reference, this is primitive 132: >> >> My proposal for the new primitive was to return the index instead of true and 0 instead of false. This change is very simple in the interpreter, but it's a lot more complicated in Cog. >> >> >> Levente > > Interesting. I thought the object memory was pretty much identical under Cog, except for Float byte order? Why is it more complicated? There's an optimization if the receiver is a MethodContext, which is complicated because of the JIT and the stack mapping IIUC. Levente > > - Bert - > >>> >>> primitiveObjectPointsTo >>> | rcvr thang lastField | >>> thang := self popStack. >>> rcvr := self popStack. >>> (self isIntegerObject: rcvr) ifTrue: [^self pushBool: false]. >>> >>> lastField := self lastPointerOf: rcvr. >>> self baseHeaderSize to: lastField by: self bytesPerWord do: >>> [:i | (self longAt: rcvr + i) = thang >>> ifTrue: [^ self pushBool: true]]. >>> self pushBool: false. >>> >>> >>> >> > > > > > |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On 09.01.2012, at 14:19, Levente Uzonyi wrote:
> How important is the performance of #pointsTo:? It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. How about implementing identityIncludes: in Array using primitive 132? That would suit my use case just fine. - Bert - |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On Mon, 9 Jan 2012, Bert Freudenberg wrote:
> On 09.01.2012, at 14:19, Levente Uzonyi wrote: > >> How important is the performance of #pointsTo:? > > > It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. > > How about implementing identityIncludes: in Array using primitive 132? That would suit my use case just fine. That's possible, but there are many other ArrayedCollections which could benefit from it (String, ByteArray, WordArray, etc). Levente > > - Bert - > > > |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On 09.01.2012, at 15:46, Levente Uzonyi wrote: > On Mon, 9 Jan 2012, Bert Freudenberg wrote: > >> On 09.01.2012, at 14:19, Levente Uzonyi wrote: >> >>> How important is the performance of #pointsTo:? >> >> >> It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. >> >> How about implementing identityIncludes: in Array using primitive 132? That would suit my use case just fine. > > That's possible, but there are many other ArrayedCollections which could benefit from it (String, ByteArray, WordArray, etc). Primitive 132 only works for pointer objects. - Bert - |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On Mon, 9 Jan 2012, Bert Freudenberg wrote:
> > On 09.01.2012, at 15:46, Levente Uzonyi wrote: > >> On Mon, 9 Jan 2012, Bert Freudenberg wrote: >> >>> On 09.01.2012, at 14:19, Levente Uzonyi wrote: >>> >>>> How important is the performance of #pointsTo:? >>> >>> >>> It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. >>> >>> How about implementing identityIncludes: in Array using primitive 132? That would suit my use case just fine. >> >> That's possible, but there are many other ArrayedCollections which could benefit from it (String, ByteArray, WordArray, etc). > > > Primitive 132 only works for pointer objects. That makes sense in most cases. :) Levente > > - Bert - > > > |
Re: The Inbox: Kernel-ul.664.mcz
|
|
In reply to this post by commits-2
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, <[hidden email]> wrote: A new version of Kernel was added to project The Inbox: best, Eliot |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On Mon, 9 Jan 2012, Eliot Miranda wrote:
> 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. You're right. I thought I can trick the system to keep an object, but making it's compact class go away, but the code is robust enough to always keep a reference to the class in the object or the compact classes array. So instances of compact classes always have at least an indirect reference to their class. > > 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. The code has many other assumptions about the system (objectmemory, garbage collector, compiler, etc), so it will have to be changed when any of those change. Levente > > On Sun, Jan 8, 2012 at 5:47 PM, <[hidden email]> 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 > |
Re: The Inbox: Kernel-ul.664.mcz
|
|
In reply to this post by Levente Uzonyi-2
On Mon, 9 Jan 2012, Bert Freudenberg wrote:
> It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. Speaking of correctness: the pointer finder does not find an object that is held only by a process. Can we fix this? - Bert - |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On Tue, 10 Jan 2012, Bert Freudenberg wrote:
> On Mon, 9 Jan 2012, Bert Freudenberg wrote: >> It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. > > > Speaking of correctness: the pointer finder does not find an object that is held only by a process. Can we fix this? Can provide an example? Levente > > - Bert - > > > > |
Pointer finder and processes
|
|
On 12.01.2012, at 15:53, Levente Uzonyi wrote: > On Tue, 10 Jan 2012, Bert Freudenberg wrote: > >> On Mon, 9 Jan 2012, Bert Freudenberg wrote: >>> It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. >> >> >> Speaking of correctness: the pointer finder does not find an object that is held only by a process. Can we fix this? > > Can provide an example? > > > Levente | object | object := {0}. [ | objRef | objRef := object. [ objRef at: 1 put: (objRef at: 1) + 1. Transcript show: objRef first; cr. (Delay forSeconds: 1) wait. ] repeat. ] forkNamed: 'Ref Test'. object explorePointers. object := nil. Initially, the explorer will find two objects pointing to the array - one is from the outer closure of the doIt, the other is the PointerExplorer itself. If you then toggle the array in the explorer, only one reference remains, from the explorer. Use the process browser to kill the process. Or before that, try to find references to the process itself from the browser's "inspect pointers" item. It seems to be held by a Semaphore, but nothing appears to hold the Semaphore. - Bert - |
Re: The Inbox: Kernel-ul.664.mcz
|
|
In reply to this post by Levente Uzonyi-2
On Thu, Jan 12, 2012 at 6:53 AM, Levente Uzonyi <[hidden email]> wrote:
If you're saying how might we do this then it seems to me we can, either by taking a snapshot of the runnable process at the start of pointer-finder, or by treating Process instances specially, querying whether a process is in the run-queue when it is encountered.
Imagine I changed the background process to allocate some object in a temporary and hold onto it. The background process would be the only referent of this object, and the only referent of the background process is the run queue.
best, Eliot |
Re: The Inbox: Kernel-ul.664.mcz
|
|
On 12.01.2012, at 18:04, Eliot Miranda wrote:
>> On Thu, Jan 12, 2012 at 6:53 AM, Levente Uzonyi <[hidden email]> wrote: >>> On Tue, 10 Jan 2012, Bert Freudenberg wrote: >>> >>> Speaking of correctness: the pointer finder does not find an object that is held only by a process. Can we fix this? >>> >> > If you're saying how might we do this then it seems to me we can, either by taking a snapshot of the runnable process at the start of pointer-finder, or by treating Process instances specially, querying whether a process is in the run-queue when it is encountered. > >> Can provide an example? > > Imagine I changed the background process to allocate some object in a temporary and hold onto it. The background process would be the only referent of this object, and the only referent of the background process is the run queue. What's the "run queue" you speak of? In my understanding, all runnable processes are held by the ProcessorScheduler, so they should be found by the pointer finder. And indeed, the idle-process is found via the #Processor global. All other processes should be in some Semaphore's wait list, and get found there. - Bert - |
Re: Pointer finder and processes
|
|
In reply to this post by Bert Freudenberg
On 12.01.2012, at 16:59, Bert Freudenberg wrote:
> On 12.01.2012, at 15:53, Levente Uzonyi wrote: > >> On Tue, 10 Jan 2012, Bert Freudenberg wrote: >> >>> On Mon, 9 Jan 2012, Bert Freudenberg wrote: >>>> It's more important to be correct than to be fast - and I agree finding pointers is its main purpose, so your fix looks like an improvement. >>> >>> >>> Speaking of correctness: the pointer finder does not find an object that is held only by a process. Can we fix this? >> >> Can provide an example? >> >> >> Levente > > > | object | > object := {0}. > [ > | objRef | > objRef := object. > [ > objRef at: 1 put: (objRef at: 1) + 1. > Transcript show: objRef first; cr. > (Delay forSeconds: 1) wait. > ] repeat. > ] forkNamed: 'Ref Test'. > object explorePointers. > object := nil. > > Initially, the explorer will find two objects pointing to the array - one is from the outer closure of the doIt, the other is the PointerExplorer itself. If you then toggle the array in the explorer, only one reference remains, from the explorer. > > Use the process browser to kill the process. Or before that, try to find references to the process itself from the browser's "inspect pointers" item. It seems to be held by a Semaphore, but nothing appears to hold the Semaphore. > > - Bert - Okay, false alarm, I understand now what's going on. The pointer finder works fine if I increase the time of the delay. The problem is that "Delay forSeconds: 1" creates a semaphore that holds onto the process, but it's gone after each second. Classical race condition - by the time the pointer finder is looking for the reference, it is already obsolete. This is even worse in my original problem which had a process polling 50 times per second. Very hard to catch that. A workaround would be to run the pointer finder on a very high priority level ... - Bert - |
Re: The Inbox: Kernel-ul.664.mcz
|
|
In reply to this post by Bert Freudenberg
On Thu, Jan 12, 2012 at 10:07 AM, Bert Freudenberg <[hidden email]> wrote:
Processor's quiescentProcessLists inst var. Its badly named because these are the runnable processes, and it is effectively a priority queue.
Right, that's my understanding too. But above you say "the pointer finder does not find an object that is held only by a process".
best, Eliot |
Re: Pointer finder and processes
|
|
In reply to this post by Bert Freudenberg
On Thu, Jan 12, 2012 at 10:23 AM, Bert Freudenberg <[hidden email]> wrote:
I doubt this will work. The system won;t be responsive while something as compute-intensive as the pointer finder runs if it does so at high priority. What if one took a snapshot of the runnable processes at high-priority, at the start of searching for pointers?
best, Eliot |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |