Hi Nicolas,
On Tue, Mar 12, 2013 at 4:54 AM, Nicolas Cellier <[hidden email]> wrote:
Primitive 119. Primitive 116 will give bugs. For example, if we have Object subclass: #A A subclass: #B A foo ^#A A bar ^self foo and we evaluate B new bar then there is an inline cache entry in A>>#bar and an entry in the first-level method lookup cache that says instances of B receiving #foo should execute A>>#foo. If we add
B foo ^#B then we haven't changed A>>#foo but the cache entries in A>>#bar and the first-level method cache for B #foo => A>>#foo must still be flushed. Make sense?
best, Eliot
|
On 14 March 2013 00:13, Eliot Miranda <[hidden email]> wrote:
> > Hi Nicolas, > > On Tue, Mar 12, 2013 at 4:54 AM, Nicolas Cellier <[hidden email]> wrote: >> >> >> As I understand it, there is a nuclear weapon >> - primitive 89 for cleaning all caches >> and more chirurgical tools: >> - primitive 116 for cleaning method cache of a single CompiledMethod >> - primitive 119 for cleaning method cache for all CompiledMethod >> corresponding to a given selector >> >> Currently, when we replace a CompiledMethod in some MethodDictionary, >> we call both primitive 116 then primitive 119. >> As I understand it from image code comments, this was to support old >> VM that would support either one or the other form. >> Modern VM (interpreter VM4.x serie, Stack or Cog) are mandatory to run >> a modern image all implement both forms, so backward support argument >> is gone. >> I think it's time to clean some dust. >> So my question is which one is necessary ? >> >> Nicolas > > > > > Primitive 119. Primitive 116 will give bugs. For example, if we have > > Object subclass: #A > > A subclass: #B > > A foo > ^#A > > A bar > ^self foo > > and we evaluate > B new bar > Hmm.. MethodDictionary>>at: key put: value "Set the value at key to be value." | index | index := self findElementOrNil: key. (self basicAt: index) ifNil: [tally := tally + 1. self basicAt: index put: key] ifNotNil: [(array at: index) flushCache]. array at: index put: value. self fullCheck. ^ value CM>>flushCache uses primitive 116. But look at the above implementation: it uses flushing only if you replacing existing method. But not if you adding new method (like overriding in a subclass), which you can simply expose with following: Object subclass: #A instanceVariableNames: '' classVariableNames: '' poolDictionaries: '' category: 'TTT' A>>foo ^ 'foo' A subclass: #B instanceVariableNames: '' classVariableNames: '' poolDictionaries: '' category: 'TTT' B>>zork ^ 'zork ' -- | x y | x := B new foo. B methodDict at: #foo put: (B>>#zork). y := B new foo. { x. y } gives: #('foo' 'foo') while expected answer is #('foo' 'zork') , of course. Also gives same result, if i do: A>>bar ^self foo and invoke. | x y | x := B new bar. B methodDict at: #foo put: (B>>#zork). y := B new bar. { x. y } #('foo' 'foo') Apparently, if we consider MethodDictionary>>at:put: as a last line of defense (against bugs), that line has a big breach, which needs to be fixed. > then there is an inline cache entry in A>>#bar and an entry in the first-level method lookup cache that says instances of B receiving #foo should execute A>>#foo. If we add > > B foo > ^#B > > then we haven't changed A>>#foo but the cache entries in A>>#bar and the first-level method cache for B #foo => A>>#foo must still be flushed. Make sense? > -- > best, > Eliot > -- Best regards, Igor Stasenko. |
as a follow-up.. given the implementation of at:put:
i expected that following would work correctly: | x y | B methodDict removeKey: #foo ifAbsent: [ ]. x := B new bar. B methodDict at: #foo put: (B>>#zork). B methodDict at: #foo put: (B>>#zork). y := B new bar. { x. y } (a second at:put: shall trigger flushing the cache).. yet it gives same answer: #('foo' 'foo') Conclusion: primitive 116 is NOT enough. While method's comment says otherwise: flushCache "Tell the interpreter to remove all references to this method from its method lookup cache, if it has one. This primitive must be called whenever a method is redefined or removed. NOTE: Only one of two selective flush methods (Symbol or CompiledMethod) needs to be used." <primitive: 116> -- Best regards, Igor Stasenko. |
2013/3/14 Igor Stasenko <[hidden email]>:
> as a follow-up.. given the implementation of at:put: > i expected that following would work correctly: > > | x y | > B methodDict removeKey: #foo ifAbsent: [ ]. > x := B new bar. > B methodDict at: #foo put: (B>>#zork). > B methodDict at: #foo put: (B>>#zork). > y := B new bar. > { x. y } > > (a second at:put: shall trigger flushing the cache).. > > yet it gives same answer: > #('foo' 'foo') > > Conclusion: primitive 116 is NOT enough. > > While method's comment says otherwise: > > flushCache > "Tell the interpreter to remove all references to this method from > its method lookup cache, if it has one. This primitive must be called > whenever a method is redefined or removed. > NOTE: Only one of two selective flush methods (Symbol or > CompiledMethod) needs to be used." > > <primitive: 116> > > -- > Best regards, > Igor Stasenko. > Yes the comment is for old VM and obsolete. But see Behavior>>basicAddSelector:withMethod: and basicRemoveMethod, they invoke primitive 119 unconditionnally. Misunderstanding in this area is dangerous, so it's time to properly document and clean if necessary, and kill false assumptions. Nicolas |
On 14 March 2013 09:29, Nicolas Cellier
<[hidden email]> wrote: > 2013/3/14 Igor Stasenko <[hidden email]>: >> as a follow-up.. given the implementation of at:put: >> i expected that following would work correctly: >> >> | x y | >> B methodDict removeKey: #foo ifAbsent: [ ]. >> x := B new bar. >> B methodDict at: #foo put: (B>>#zork). >> B methodDict at: #foo put: (B>>#zork). >> y := B new bar. >> { x. y } >> >> (a second at:put: shall trigger flushing the cache).. >> >> yet it gives same answer: >> #('foo' 'foo') >> >> Conclusion: primitive 116 is NOT enough. >> >> While method's comment says otherwise: >> >> flushCache >> "Tell the interpreter to remove all references to this method from >> its method lookup cache, if it has one. This primitive must be called >> whenever a method is redefined or removed. >> NOTE: Only one of two selective flush methods (Symbol or >> CompiledMethod) needs to be used." >> >> <primitive: 116> >> >> -- >> Best regards, >> Igor Stasenko. >> > > Yes the comment is for old VM and obsolete. > But see Behavior>>basicAddSelector:withMethod: and basicRemoveMethod, > they invoke primitive 119 unconditionnally. > I am for putting this critical behavior into method dictionary.. then there is guarantee that no matter how you play with class(es) it will flush things properly. > Misunderstanding in this area is dangerous, so it's time to properly > document and clean if necessary, and kill false assumptions. > > Nicolas > -- Best regards, Igor Stasenko. |
Free forum by Nabble | Edit this page |