[Pharo-project] [update 1.1] #11126 - 27 (final methodTrailer cool cleanup
BIG THANKS igor!!! 11127 ----- - Issue 1690: New Method Trailer part 7 (cs 9)
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.) Levente
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Thanks I will do that as soon as I can. Stef On Jan 4, 2010, at 2:45 AM, Levente Uzonyi wrote:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
http://code.google.com/p/pharo/issues/detail?id=1738
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
hmm. i thought i fixed this one. 2010/1/4 Levente Uzonyi <leves@elte.hu>:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ 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.
Igor what is the new way of doing methodAndNode := self compile: source classified: nil notifying: nil trailer: #(0 0 0 0) ifFail: [^nil]. in addTraitSelector: aSymbol withMethod: aCompiledMethod "Add aMethod with selector aSymbol to my methodDict. aMethod must not be defined locally." | source methodAndNode | self assert: [(self includesLocalSelector: aSymbol) not]. self ensureLocalSelectors. source := aCompiledMethod getSourceReplacingSelectorWith: aSymbol. methodAndNode := self compile: source classified: nil notifying: nil trailer: #(0 0 0 0) ifFail: [^nil]. methodAndNode method putSource: source fromParseNode: methodAndNode node inFile: 2 withPreamble: [:f | f cr; nextPut: $!; nextChunkPut: 'Trait method'; cr]. self basicAddSelector: aSymbol withMethod: methodAndNode method In squeak and pharo the following code is used and I was wondering if the API (explicit ue of trailer: or bytes (maybe a trailerObject is passed - I did not check) is good. I thought that this was the job of the CompilerMethodTrailer to know the bytes. compile: code classified: category notifying: requestor trailer: bytes ifFail: failBlock "Compile code without logging the source in the changes file" | methodNode | methodNode := self compilerClass new compile: code in: self classified: category notifying: requestor ifFail: failBlock. ^ CompiledMethodWithNode generateMethodFromNode: methodNode trailer: bytes. CompiledMethodWithNode>>generateMethodFromNode: aMethodNode trailer: bytes ^ self method: (aMethodNode generate: bytes) node: aMethodNode. Stef
hmm. i thought i fixed this one.
2010/1/4 Levente Uzonyi <leves@elte.hu>:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ 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.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
methodAndNode := self compile: source classified: nil notifying: nil trailer: self defaultMethodTrailer ifFail: [^nil].
?? On Jan 4, 2010, at 3:19 PM, Stéphane Ducasse wrote:
Igor
what is the new way of doing
methodAndNode := self compile: source classified: nil notifying: nil trailer: #(0 0 0 0) ifFail: [^nil].
in addTraitSelector: aSymbol withMethod: aCompiledMethod "Add aMethod with selector aSymbol to my methodDict. aMethod must not be defined locally."
| source methodAndNode | self assert: [(self includesLocalSelector: aSymbol) not]. self ensureLocalSelectors.
source := aCompiledMethod getSourceReplacingSelectorWith: aSymbol. methodAndNode := self compile: source classified: nil notifying: nil trailer: #(0 0 0 0) ifFail: [^nil]. methodAndNode method putSource: source fromParseNode: methodAndNode node inFile: 2 withPreamble: [:f | f cr; nextPut: $!; nextChunkPut: 'Trait method'; cr].
self basicAddSelector: aSymbol withMethod: methodAndNode method
In squeak and pharo the following code is used and I was wondering if the API (explicit ue of trailer: or bytes (maybe a trailerObject is passed - I did not check) is good. I thought that this was the job of the CompilerMethodTrailer to know the bytes.
compile: code classified: category notifying: requestor trailer: bytes ifFail: failBlock "Compile code without logging the source in the changes file"
| methodNode | methodNode := self compilerClass new compile: code in: self classified: category notifying: requestor ifFail: failBlock. ^ CompiledMethodWithNode generateMethodFromNode: methodNode trailer: bytes.
CompiledMethodWithNode>>generateMethodFromNode: aMethodNode trailer: bytes ^ self method: (aMethodNode generate: bytes) node: aMethodNode.
Stef
hmm. i thought i fixed this one.
2010/1/4 Levente Uzonyi <leves@elte.hu>:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ 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.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
2010/1/4 Stéphane Ducasse <stephane.ducasse@inria.fr>:
methodAndNode  := self        compile: source        classified: nil        notifying: nil        trailer: self defaultMethodTrailer        ifFail: [^nil].
??
yup. -- Best regards, Igor Stasenko AKA sig.
Ok I will fix that now. On Jan 4, 2010, at 3:27 PM, Igor Stasenko wrote:
2010/1/4 Stéphane Ducasse <stephane.ducasse@inria.fr>:
methodAndNode := self compile: source classified: nil notifying: nil trailer: self defaultMethodTrailer ifFail: [^nil].
??
yup.
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Igor when I try to recompile TPureBehavior>>addTraitSelector: I get a DNU Array>>createMethod:header: because the system uses itself when I add a method to a trait. This morning I did something and it worked but now I do not remember and I'm thinking how to turn around the problem. Do you have an idea? Stef
methodAndNode := self compile: source classified: nil notifying: nil trailer: self defaultMethodTrailer ifFail: [^nil].
??
yup.
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
As a solution I was planning to redefine Behavior>>addTraitSelector, TraitBehavior>>addTraitSelector and then redefine it TPureBehavior then remove Behavior>>addTraitSelector, TraitBehavior>>addTraitSelector I imagine that it can work now I will try to see Stef On Jan 4, 2010, at 8:54 PM, Stéphane Ducasse wrote:
Igor when I try to recompile TPureBehavior>>addTraitSelector:
I get a DNU Array>>createMethod:header: because the system uses itself when I add a method to a trait.
This morning I did something and it worked but now I do not remember and I'm thinking how to turn around the problem. Do you have an idea?
Stef
methodAndNode := self compile: source classified: nil notifying: nil trailer: self defaultMethodTrailer ifFail: [^nil].
??
yup.
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
2010/1/4 Stéphane Ducasse <stephane.ducasse@inria.fr>:
Igor when I try to recompile TPureBehavior>>addTraitSelector:
I get a DNU Array>>createMethod:header: because the system uses itself when I add a method to a trait.
This morning I did something and it worked but now I do not remember and I'm thinking how to turn around the problem. Do you have an idea?
you can restore the ArrayedCollection>>createMethod:header: method, just temporary. So you will be able to produce methods using array of bytes as trailer. But it is strange.. i remember i fixed that method (addTraitSelector:) in pharo changesets. Or maybe it was something else.
Stef
methodAndNode  := self        compile: source        classified: nil        notifying: nil        trailer: self defaultMethodTrailer        ifFail: [^nil].
??
yup.
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ 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.
yes this is probably easier. Glad I was looking for lenses :) I will do it now. Stef On Jan 4, 2010, at 9:57 PM, Igor Stasenko wrote:
2010/1/4 Stéphane Ducasse <stephane.ducasse@inria.fr>:
Igor when I try to recompile TPureBehavior>>addTraitSelector:
I get a DNU Array>>createMethod:header: because the system uses itself when I add a method to a trait.
This morning I did something and it worked but now I do not remember and I'm thinking how to turn around the problem. Do you have an idea?
you can restore the ArrayedCollection>>createMethod:header: method, just temporary. So you will be able to produce methods using array of bytes as trailer.
But it is strange.. i remember i fixed that method (addTraitSelector:) in pharo changesets. Or maybe it was something else.
Stef
methodAndNode := self compile: source classified: nil notifying: nil trailer: self defaultMethodTrailer ifFail: [^nil].
??
yup.
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ 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.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
2010/1/4 Stéphane Ducasse <stephane.ducasse@inria.fr>:
Igor
what is the new way of doing
    methodAndNode  := self         compile: source         classified: nil         notifying: nil         trailer: #(0 0 0 0)         ifFail: [^nil].
in addTraitSelector: aSymbol withMethod: aCompiledMethod     "Add aMethod with selector aSymbol to my     methodDict. aMethod must not be defined locally."
    | source methodAndNode |     self assert: [(self includesLocalSelector: aSymbol) not].     self ensureLocalSelectors.
    source := aCompiledMethod getSourceReplacingSelectorWith: aSymbol.     methodAndNode  := self         compile: source         classified: nil         notifying: nil         trailer: #(0 0 0 0)         ifFail: [^nil].     methodAndNode method putSource: source fromParseNode: methodAndNode node inFile: 2         withPreamble: [:f | f cr; nextPut: $!; nextChunkPut: 'Trait method'; cr].
    self basicAddSelector: aSymbol withMethod: methodAndNode method
In squeak and pharo the following code is used and I was wondering if the API (explicit ue of trailer: or bytes (maybe a trailerObject is passed - I did not check) is good. I thought that this was the job of the CompilerMethodTrailer to know the bytes.
compile: code classified: category notifying: requestor trailer: bytes ifFail: failBlock     "Compile code without logging the source in the changes file"
    | methodNode |     methodNode  := self compilerClass new                 compile: code                 in: self                 classified: category                 notifying: requestor                 ifFail: failBlock.     ^ CompiledMethodWithNode generateMethodFromNode: methodNode trailer: bytes.
CompiledMethodWithNode>>generateMethodFromNode: aMethodNode trailer: bytes     ^ self method: (aMethodNode generate: bytes) node: aMethodNode.
Stef
sorry for naming confusion . It deserves a proper 'trailer' name in those methods instead of 'bytes'. And yes, there should be no bytes anymore.. so, anywhere you see them - you should nuke em.
hmm. i thought i fixed this one.
-- Best regards, Igor Stasenko AKA sig.
On Jan 4, 2010, at 2:45 57AM, Levente Uzonyi wrote:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
CompiledMethod>>storeOn: also needs to be updated, (Compiler evaluate: aCompiledMethod storeString) does not reconstruct the original trailer. Caught by ArrayTest >> testComplexIsSelfEvaluating, of all places :) Cheers, Henry
2010/1/4 Henrik Johansen <henrik.s.johansen@veloxit.no>:
On Jan 4, 2010, at 2:45 57AM, Levente Uzonyi wrote:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
CompiledMethod>>storeOn: also needs to be updated,
(Compiler evaluate: aCompiledMethod storeString) does not reconstruct the original trailer.
The only way how you should be allowed to reconstruct a compiled method from string, is compilation, otherwise you risking crashing the system. But compilation _always_ creating a new artifact - a compiled method , and picking an appropriate trailer for it. The way, how CompiledMethod>>storeOn: does, seems an abuse, as to me, because i won't bet on safety of keeping a methods in such format, even for existing images. For instance, you could store a method , then condense changes, and then if you attempt to restore it, you will end up with having a sourcePointer pointing to invalid .changes file location. So, i rather prefer seeing this method implemented like following: CompiledMethod>>storeOn: aStream ^ self shoulNotBeImplemented
Caught by ArrayTest >> testComplexIsSelfEvaluating, of all places :)
Cheers, Henry _______________________________________________ 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.
If the method is installed, I would only store a reference (MyClass>>#myMethod). That's what is done on Class, they are stored by reference: String streamContents: [:strm | Array storeOn: strm] -> 'Array' Otherwise, if the method is not installed, I would not store a trailer pointing to some source somewhere indeed, but why not a trailer encoding compressed temp vars or direct source... Anyway, is the trailer a relevant point of comparison ? Nicolas 2010/1/4 Igor Stasenko <siguctua@gmail.com>:
2010/1/4 Henrik Johansen <henrik.s.johansen@veloxit.no>:
On Jan 4, 2010, at 2:45 57AM, Levente Uzonyi wrote:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
CompiledMethod>>storeOn: also needs to be updated,
(Compiler evaluate: aCompiledMethod storeString) does not reconstruct the original trailer.
The only way how you should be allowed to reconstruct a compiled method from string, is compilation, otherwise you risking crashing the system. But compilation _always_ creating a new artifact - a compiled method , and picking an appropriate trailer for it.
The way, how CompiledMethod>>storeOn: does, seems an abuse, as to me, because i won't bet on safety of keeping a methods in such format, even for existing images. For instance, you could store a method , then condense changes, and then if you attempt to restore it, you will end up with having a sourcePointer pointing to invalid .changes file location.
So, i rather prefer seeing this method implemented like following:
CompiledMethod>>storeOn: aStream  ^ self shoulNotBeImplemented
Caught by ArrayTest >> testComplexIsSelfEvaluating, of all places :)
Cheers, Henry _______________________________________________ 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.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
2010/1/4 Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com>:
If the method is installed, I would only store a reference (MyClass>>#myMethod).
+1
That's what is done on Class, they are stored by reference:
String streamContents: [:strm | Array storeOn: strm] -> 'Array'
Otherwise, if the method is not installed, I would not store a trailer pointing to some source somewhere indeed, but why not a trailer encoding compressed temp vars or direct source...
because you could store the source code instead.
Anyway, is the trailer a relevant point of comparison ?
i am against storing the compiled method in any other form than source code, except referential form, like 'Array >> #at: ' , you mentioned. So, either way, after restoring you getting an already existing CompiledMethod instance, or fresh one, which just compiled from stored source code. We just should not allow a potentially dangerous ways of manipulating a binary data, stored in compiled method, including trailer and bytecodes. Because they are subject of change (if we observe them over long period ;)
Nicolas
2010/1/4 Igor Stasenko <siguctua@gmail.com>:
2010/1/4 Henrik Johansen <henrik.s.johansen@veloxit.no>:
On Jan 4, 2010, at 2:45 57AM, Levente Uzonyi wrote:
On Wed, 30 Dec 2009, Stéphane Ducasse wrote:
BIG THANKS igor!!!
11127 -----
- Issue 1690: New Method Trailer part 7 (cs 9)
There are still some issues. Some methods (for example TPureBehavior >> #addTraitSelector:withMethod:) still use the old trailer bytes #(0 0 0 0). These should be removed asap (I had to make some surgery with #become: to compile the new version.)
Levente
CompiledMethod>>storeOn: also needs to be updated,
(Compiler evaluate: aCompiledMethod storeString) does not reconstruct the original trailer.
The only way how you should be allowed to reconstruct a compiled method from string, is compilation, otherwise you risking crashing the system. But compilation _always_ creating a new artifact - a compiled method , and picking an appropriate trailer for it.
The way, how CompiledMethod>>storeOn: does, seems an abuse, as to me, because i won't bet on safety of keeping a methods in such format, even for existing images. For instance, you could store a method , then condense changes, and then if you attempt to restore it, you will end up with having a sourcePointer pointing to invalid .changes file location.
So, i rather prefer seeing this method implemented like following:
CompiledMethod>>storeOn: aStream  ^ self shoulNotBeImplemented
Caught by ArrayTest >> testComplexIsSelfEvaluating, of all places :)
Cheers, Henry _______________________________________________ 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.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ 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.
participants (5)
-
Henrik Johansen -
Igor Stasenko -
Levente Uzonyi -
Nicolas Cellier -
Stéphane Ducasse