2009/10/7 Johan Brichau <johan.brichau@uclouvain.be>:

This fixes it. It seems the last 2 literals in each method are selector and classbinding.

So now I will try to submit it as a pharo-process-compatible patch too (I should learn someday anyway... :-)

---- CompiledMethod>>refersToLiteral: aLiteral        "Answer true if any literal in this method is literal, even if embedded in array structure or within its pragmas."        "only iterate to numLiterals - 1 , as the last has the classBinding and method selector"        2                to: self numLiterals - 1                do:                        [ :index |                        | literal |                        literal := self objectAt: index.                        literal == aLiteral ifTrue: [ ^ true ].                        (literal refersToLiteral: aLiteral) ifTrue: [ ^ true ] ].        ^ false

Maybe its worth to change #literalsDo: method to not iterate over class and method selector? And then use it in #refersToLiteral: ? -- Best regards, Igor Stasenko AKA sig.

2009/10/7 Johan Brichau <johan.brichau@uclouvain.be>:

On 07 Oct 2009, at 15:19, Igor Stasenko wrote:

...

Maybe its worth to change #literalsDo: method to not iterate over class and method selector? And then use it in #refersToLiteral: ?

Actually, I just noticed after publishing the fix that the last-but- one position can contain either the method selector -or- the method's pragma (which in turn will hold the method's selector).

So, since the #refersToLiteral: method is supposed to return also references to literals from the pragma's (see somment) I had to adapt the fix to take the last-but-one position as a special case... rather than simply excluding it.

Okay, if you need to visit pragmas as well, then why don't do it: CompiledMethod>>refersToLiteral: aLiteral self literalsDo: [ blablah ]. self pragmas do: [ blah blah ] ^ false which makes this method immune from any future changes in CompiledMethod format. So, we don't need to think about 'fixing' it again and again, each time we need to hack things.

---------------------------- Johan Brichau johan.brichau@uclouvain.be

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

-- Best regards, Igor Stasenko AKA sig.

2009/10/7 Lukas Renggli <renggli@gmail.com>:

...

Okay, if you need to visit pragmas as well, then why don't do it:

Yes, this is important. Otherwise you can't browser for references to a Pragma anymore.

...

CompiledMethod>>refersToLiteral: aLiteral

 self literalsDo: [ blablah ].  self pragmas do: [ blah blah ]

      ^ false

which makes this method immune from any future changes in CompiledMethod format. So, we don't need to think about 'fixing' it again and again, each time we need to hack things.

This would introduce a major performance bottleneck for commonly used Smalltalk features like senders/implementors/references.

Well, before stating that, we should measure it. My point, that we should keep a minimum methods, which is aware of a deep internals of method's format. Then it would be easier to maintain it.

Lukas

-- Lukas Renggli http://www.lukas-renggli.ch

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

-- Best regards, Igor Stasenko AKA sig.

On 07 Oct 2009, at 21:38, Stéphane Ducasse wrote:

I want benchs :)

Stef

Here they are: Time millisecondsToRun: [1 to: 10 do:[:i | SystemNavigation new allCallsOn: #add: ]]. "current: 2234 2240 2261" "refactored: 2806 2799 2816" Time millisecondsToRun: [1 to: 10 do:[:i | SystemNavigation new allCallsOn: #foobar ]]. "current: 2166 2216 2159" "refactored: 2826 2790 2933" So, an overhead of 20-30%. However, making that refactoring will probably also trigger a lot of changes elsewhere. The current #literalsDo: method iterates over header, method properties and class binding too and it is sent by external methods. In addition, there is the #hasLiteral:, #hasLiteralThorough: and #sendsSelector: methods on CompiledMethod that all perform roughly identical tasks but it is far from clear to me who is expected to do what, especially since the comments are sometimes not in sync with what the method actually does. It seems like this part of the code is another candidate for cleanup. ---------------------------- Johan Brichau johan.brichau@uclouvain.be

On 07 Oct 2009, at 21:55, Stéphane Ducasse wrote:

did you open a ticket?

Yes, I did: 1288 but I will probably send in a cleaner fix today (instead of the two previous ones... sorry about that!) ---------------------------- Johan Brichau johan.brichau@uclouvain.be