On Mon, Jan 04, 2010 at 05:15:28PM +0100, Andreas Raab wrote:
> Hi Jannik -
> 
> A couple of comments. First, If wordSize is a constant, put it into a 
> class var. That's the best practice pattern for dealing with constants 
> and I see no reason why one would opt to use an ivar for a value that 
> never changes. See for example the EndianCache in SmalltalkImage and 
> other use of constants throughout the system.
> 
> As for caching, if I understand your description correctly, then system 
> tracer is storing the correct value for the traced image. When the image 
> starts, your cache code invalidates the known to be correct value which 
> is later lazily filled in with the same value again. So what observable 
> effect does your cache invalidation have?
> 
> #startUp isn't the first method sent, not by a very long shot - in 
> particular when you mess with the execution machinery you have to be 
> aware that a method like #initialPC might be called before you ever get 
> to the point where you invalidate that cache (I'm not sure if this can 
> happen in this concrete case). In any case you should trace through the 
> startup sequence to find out just how much other code is executed before 
> getting there.

John and/or Bert previously pointed out that there is no need to ever
set the cached value from the image, so this would be done from a system
tracer only (and yes this should have a comment).

> My recommendations would be to make WordSize a class variable, leave the 
> lazy initialization in (it might be helpful during install etc) but add 
> a nice comment explaining that only system tracer ever modifies that 
> value when a 32/64 bit image is written. And leave out the pointless 
> cache invalidation :-)

I think that the change set on Mantis 7430 does what you describe. This
puts the class variable in SystemDictionary in order to retain the current
"Smalltalk wordSize" idiom, and uses the original Dan Ingalls #initialPC
implementation from the "dist3" 64-bit image.

It's a small change, so I'll copy it here:

'From Squeak3.10.2 of 16 December 2009 [latest update: #8496] on 18 December 2009 at 6:08:11 pm'!
"Change Set:		Smalltalk-wordSize-dtl-M7430
Date:			18 December 2009
Author:			David T. Lewis

Cache Smalltalk wordSize in class var in SystemDictionary..

Update CompiledMethod>>initialPC to use #wordSize. This is the method as implemented in the original 64-bit image (author di)."!

IdentityDictionary subclass: #SystemDictionary
	instanceVariableNames: 'cachedClassNames'
	classVariableNames: 'LastImageName LastQuitLogPosition LowSpaceProcess LowSpaceSemaphore MemoryHogs ShutDownList SpecialSelectors StartUpList StartupStamp SystemChanges WordSize'
	poolDictionaries: ''
	category: 'System-Support'!

!CompiledMethod methodsFor: 'accessing' stamp: 'di 6/29/2004 12:28'!
initialPC
	"Answer the program counter for the receiver's first bytecode."

	^ (self numLiterals + 1) * Smalltalk wordSize + 1
! !


!SystemDictionary methodsFor: 'sources, change log' stamp: 'dtl 12/18/2009 00:32'!
wordSize
	"Answer the size (in bytes) of an object pointer."
	"Smalltalk wordSize"

	^ WordSize ifNil: [WordSize := [SmalltalkImage current vmParameterAt: 40] on: Error do: [4]]! !