The Trunk: Tests-dtl.194.mcz
Locked
 
|
David T. Lewis uploaded a new version of Tests to project The Trunk:
http://source.squeak.org/trunk/Tests-dtl.194.mcz ==================== Summary ==================== Name: Tests-dtl.194 Author: dtl Time: 29 March 2013, 10:13:41.368 am UUID: ba96f5b8-f215-4045-a16f-473c8dde8760 Ancestors: Tests-eem.193 Merge Tests-dtl.193 Background: http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-March/169911.html Two tests that illustrate the limit to number of literals in a compiled method. Both tests pass on Squeak 4.4. One of the tests fails in the most recent Squeak trunk. CompilerTest>>testMaxLiteralsWithClassReferenceInClosure passes in the image at http://build.squeak.org/job/SqueakTrunk/212/ and fails in later updates to trunk. =============== Diff against Tests-eem.193 =============== Item was added: + ----- Method: CompilerTest>>testMaxLiterals (in category 'limits') ----- + testMaxLiterals + "Document the maximum number of literals in a compiled method" + + | maxLiterals stringThatCanBeCompiled stringWithOneTooManyLiterals | + maxLiterals := 249. + stringThatCanBeCompiled := '{ ', (String streamContents: [:strm | + 1 to: maxLiterals do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. + stringWithOneTooManyLiterals := '{ ', (String streamContents: [:strm | + 1 to: maxLiterals + 1 do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. + self shouldnt: [Compiler evaluate: stringThatCanBeCompiled logged: false] raise: Error. + self should: (Compiler evaluate: stringThatCanBeCompiled logged: false) size = maxLiterals. + + "If the following test fails, it means that the limit has been raised or eliminated, + and this test should be updated to reflect the improvement." + self should: [Compiler evaluate: stringWithOneTooManyLiterals logged: false] raise: Error. + ! Item was added: + ----- Method: CompilerTest>>testMaxLiteralsWithClassReferenceInClosure (in category 'limits') ----- + testMaxLiteralsWithClassReferenceInClosure + "Document the maximum number of literals in a compiled method. A class + reference in a closure reduces the maximum literals." + + | maxLiterals stringThatCanBeCompiled stringWithOneTooManyLiterals | + maxLiterals := 244. + stringThatCanBeCompiled := '[ DateAndTime now. Date today. Time ]. { ', + (String streamContents: [:strm | + 1 to: maxLiterals do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. + stringWithOneTooManyLiterals := '[ DateAndTime now. Date today. Time ]. { ', + (String streamContents: [:strm | + 1 to: maxLiterals + 1 do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. + self shouldnt: [Compiler evaluate: stringThatCanBeCompiled logged: false] raise: Error. + self should: (Compiler evaluate: stringThatCanBeCompiled logged: false) size = maxLiterals. + + "If the following test fails, it means that the limit has been raised or eliminated, + and this test should be updated to reflect the improvement." + self should: [Compiler evaluate: stringWithOneTooManyLiterals logged: false] raise: Error. + ! |
Re: The Trunk: Tests-dtl.194.mcz
|
|
I moved these two new tests from Inbox to Trunk. One test fails on recent
trunk images. I do not know if this is a bug or a feature, but I cannot explain the regression so I don't want to assume it's a feature. Dave On Fri, Mar 29, 2013 at 02:14:02PM +0000, [hidden email] wrote: > David T. Lewis uploaded a new version of Tests to project The Trunk: > http://source.squeak.org/trunk/Tests-dtl.194.mcz > > ==================== Summary ==================== > > Name: Tests-dtl.194 > Author: dtl > Time: 29 March 2013, 10:13:41.368 am > UUID: ba96f5b8-f215-4045-a16f-473c8dde8760 > Ancestors: Tests-eem.193 > > Merge Tests-dtl.193 > > Background: http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-March/169911.html > > Two tests that illustrate the limit to number of literals in a compiled method. Both tests pass on Squeak 4.4. One of the tests fails in the most recent Squeak trunk. > > CompilerTest>>testMaxLiteralsWithClassReferenceInClosure passes in the image at > http://build.squeak.org/job/SqueakTrunk/212/ and fails in later updates to trunk. > > =============== Diff against Tests-eem.193 =============== > > Item was added: > + ----- Method: CompilerTest>>testMaxLiterals (in category 'limits') ----- > + testMaxLiterals > + "Document the maximum number of literals in a compiled method" > + > + | maxLiterals stringThatCanBeCompiled stringWithOneTooManyLiterals | > + maxLiterals := 249. > + stringThatCanBeCompiled := '{ ', (String streamContents: [:strm | > + 1 to: maxLiterals do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. > + stringWithOneTooManyLiterals := '{ ', (String streamContents: [:strm | > + 1 to: maxLiterals + 1 do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. > + self shouldnt: [Compiler evaluate: stringThatCanBeCompiled logged: false] raise: Error. > + self should: (Compiler evaluate: stringThatCanBeCompiled logged: false) size = maxLiterals. > + > + "If the following test fails, it means that the limit has been raised or eliminated, > + and this test should be updated to reflect the improvement." > + self should: [Compiler evaluate: stringWithOneTooManyLiterals logged: false] raise: Error. > + ! > > Item was added: > + ----- Method: CompilerTest>>testMaxLiteralsWithClassReferenceInClosure (in category 'limits') ----- > + testMaxLiteralsWithClassReferenceInClosure > + "Document the maximum number of literals in a compiled method. A class > + reference in a closure reduces the maximum literals." > + > + | maxLiterals stringThatCanBeCompiled stringWithOneTooManyLiterals | > + maxLiterals := 244. > + stringThatCanBeCompiled := '[ DateAndTime now. Date today. Time ]. { ', > + (String streamContents: [:strm | > + 1 to: maxLiterals do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. > + stringWithOneTooManyLiterals := '[ DateAndTime now. Date today. Time ]. { ', > + (String streamContents: [:strm | > + 1 to: maxLiterals + 1 do: [:e | strm nextPutAll: '''', e asString, '''', ' . ']]), '}'. > + self shouldnt: [Compiler evaluate: stringThatCanBeCompiled logged: false] raise: Error. > + self should: (Compiler evaluate: stringThatCanBeCompiled logged: false) size = maxLiterals. > + > + "If the following test fails, it means that the limit has been raised or eliminated, > + and this test should be updated to reflect the improvement." > + self should: [Compiler evaluate: stringWithOneTooManyLiterals logged: false] raise: Error. > + ! > |
Re: The Trunk: Tests-dtl.194.mcz
|
|
On 29 March 2013 14:17, David T. Lewis <[hidden email]> wrote:
> > I moved these two new tests from Inbox to Trunk. One test fails on recent > trunk images. I do not know if this is a bug or a feature, but I cannot > explain the regression so I don't want to assume it's a feature. > > Dave http://build.squeak.org/job/SqueakTrunk/237/testReport/junit/Tests.Compiler/CompilerTest/testMaxLiteralsWithClassReferenceInClosure/ shows the test failure. |
Re: The Trunk: Tests-dtl.194.mcz
|
|
On Fri, Mar 29, 2013 at 03:04:20PM +0000, Frank Shearar wrote:
> On 29 March 2013 14:17, David T. Lewis <[hidden email]> wrote: > > > > I moved these two new tests from Inbox to Trunk. One test fails on recent > > trunk images. I do not know if this is a bug or a feature, but I cannot > > explain the regression so I don't want to assume it's a feature. > > > > Dave > > http://build.squeak.org/job/SqueakTrunk/237/testReport/junit/Tests.Compiler/CompilerTest/testMaxLiteralsWithClassReferenceInClosure/ > > shows the test failure. The actual change to the image that causes this test failure occurred between SqueakTrunk/212 and SqueakTrunk/213. In other words, if this test had been part of the test suite all along, then you would have seen the failure appear in 213. I suspect that this is a side effect of class lookups associated with the Environments changes, but I cannot say if it is a bug or an explainable variation. Either way it's good to note the change, and we can either fix the bug or fix the test as needed. Dave |
LiteralVariableNode>>sizeCodeFor
|
|
On Fri, Mar 29, 2013 at 11:21:00AM -0400, David T. Lewis wrote:
> On Fri, Mar 29, 2013 at 03:04:20PM +0000, Frank Shearar wrote: > > On 29 March 2013 14:17, David T. Lewis <[hidden email]> wrote: > > > > > > I moved these two new tests from Inbox to Trunk. One test fails on recent > > > trunk images. I do not know if this is a bug or a feature, but I cannot > > > explain the regression so I don't want to assume it's a feature. > > > > > > Dave > > > > http://build.squeak.org/job/SqueakTrunk/237/testReport/junit/Tests.Compiler/CompilerTest/testMaxLiteralsWithClassReferenceInClosure/ > > > > shows the test failure. > > The actual change to the image that causes this test failure occurred between > SqueakTrunk/212 and SqueakTrunk/213. In other words, if this test had been > part of the test suite all along, then you would have seen the failure appear > in 213. > > I suspect that this is a side effect of class lookups associated with the Environments > changes, but I cannot say if it is a bug or an explainable variation. Either way > it's good to note the change, and we can either fix the bug or fix the test > as needed. (self flag: #dubious and: [self flag: #bogus]) ifTrue: [self askForGuidance] LiteralVariableNode>>sizeCodeForStore: is flagged thus, which I suspect is somehow related to the apparently incorrect warning about too many literals. Reducing to a smaller test case, compiling 'Time now' yields a parse tree with LiteralVariableNode {Time} that has code -4 which causes dual reserving in sizeCodeForValue: and this in turn leads to a mis-counting of literals in the encoder. In an earlier version of trunk, the same LiteralVariableNode has a non-negative code value that does not produce the dual reserving in sizeCodeForValue: Dave |
Re: LiteralVariableNode>>sizeCodeFor
|
|
Hi David,
On Tue, Apr 2, 2013 at 7:46 PM, David T. Lewis <[hidden email]> wrote: On Fri, Mar 29, 2013 at 11:21:00AM -0400, David T. Lewis wrote: I doubt it. How to reproduce the incorrect warning? The point about LiteralVariableNode>>sizeCodeForStore: (& LiteralVariableNode>>emitCodeForStore:encoder:) is that they don't do what's advertised. i.e. they don't leave the value of the expression that is stored on top of stack unless the special write binding answers the expression in response to #value:. So if one were to write
v := SomeSpecialBinding := expr v would end up holding SomeSpecialBinding, not expr. This could perhaps be corrected by ensuring that all classes that implement isSpecialWriteBinding also implement #value: to answer the argument, not the receiver. e.g. instead of
Binding methods for accessing value: anObject (AttemptToWriteReadOnlyGlobal signal: 'Cannot store into read-only bindings')
ifTrue: [value := anObject] we use value: anObject (AttemptToWriteReadOnlyGlobal signal: 'Cannot store into read-only bindings')
ifTrue: [value := anObject]. ^anObject but that's tricky.
The problem for the code generator is that there's no swap operation. e.g. the following would work: stack before: expr first half of code:
dup push binding swap stack during: expr binding expr second half of code send #value:
pop stack after expr but that's a) a new bytecode and b) a lot of bytes to do a store. A different approach would be to rewrite emit because it knows the stack pointer to write something like
push binding push temp N (where N is the offset of expr on the stack) send value: I think this is the correct approach. It might require fixing the decompiler, but it shouldn't be hard. I'll have a go. In any case we need a test that ensures that
| v expr | expr := Object new. v := SomeSpecialBinding := expr. self assert: v == expr
best, Eliot
|
Re: LiteralVariableNode>>sizeCodeFor
|
|
In reply to this post by David T. Lewis
OK, the #bogus flags are gone :) Colin write the tests and I fixed the compiler. I've also fixed the literal duplication issue (which indeed is a side-effect of introducing Environments). But in doing so the Decompiler is broken for v := Binding := expr. Kudos to anyone who can fix this (I'm busy fixing a horrible bug of my own).
On Tue, Apr 2, 2013 at 7:46 PM, David T. Lewis <[hidden email]> wrote: On Fri, Mar 29, 2013 at 11:21:00AM -0400, David T. Lewis wrote: best, Eliot
|
Re: LiteralVariableNode>>sizeCodeFor
|
|
Yay! And CompilerTest>>testMaxLiteralsWithClassReferenceInClosure is
green now :) Don't get me wrong, I really appreciated those #bogus flags. Seriously. It was an excellent clue that I should stop random debugging and ask for help from someone who might actually know what they were doing ;-) Dave On Wed, Apr 03, 2013 at 03:34:59PM -0700, Eliot Miranda wrote: > OK, the #bogus flags are gone :) Colin write the tests and I fixed the > compiler. I've also fixed the literal duplication issue (which indeed is a > side-effect of introducing Environments). But in doing so the Decompiler > is broken for v := Binding := expr. Kudos to anyone who can fix this (I'm > busy fixing a horrible bug of my own). > > On Tue, Apr 2, 2013 at 7:46 PM, David T. Lewis <[hidden email]> wrote: > > > On Fri, Mar 29, 2013 at 11:21:00AM -0400, David T. Lewis wrote: > > > On Fri, Mar 29, 2013 at 03:04:20PM +0000, Frank Shearar wrote: > > > > On 29 March 2013 14:17, David T. Lewis <[hidden email]> wrote: > > > > > > > > > > I moved these two new tests from Inbox to Trunk. One test fails on > > recent > > > > > trunk images. I do not know if this is a bug or a feature, but I > > cannot > > > > > explain the regression so I don't want to assume it's a feature. > > > > > > > > > > Dave > > > > > > > > > > http://build.squeak.org/job/SqueakTrunk/237/testReport/junit/Tests.Compiler/CompilerTest/testMaxLiteralsWithClassReferenceInClosure/ > > > > > > > > shows the test failure. > > > > > > The actual change to the image that causes this test failure occurred > > between > > > SqueakTrunk/212 and SqueakTrunk/213. In other words, if this test had > > been > > > part of the test suite all along, then you would have seen the failure > > appear > > > in 213. > > > > > > I suspect that this is a side effect of class lookups associated with > > the Environments > > > changes, but I cannot say if it is a bug or an explainable variation. > > Either way > > > it's good to note the change, and we can either fix the bug or fix the > > test > > > as needed. > > > > (self flag: #dubious and: [self flag: #bogus]) ifTrue: [self > > askForGuidance] > > > > LiteralVariableNode>>sizeCodeForStore: is flagged thus, which I suspect > > is somehow related to the apparently incorrect warning about too many > > literals. > > > > Reducing to a smaller test case, compiling 'Time now' yields a parse tree > > with LiteralVariableNode {Time} that has code -4 which causes dual > > reserving > > in sizeCodeForValue: and this in turn leads to a mis-counting of literals > > in the encoder. > > > > In an earlier version of trunk, the same LiteralVariableNode has a > > non-negative > > code value that does not produce the dual reserving in sizeCodeForValue: > > > > Dave > > > > > > > > > -- > best, > Eliot > |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |