Re: [squeak-dev] Re: [Vm-dev] Bug in writing compressed stream when saving an mcz (was: New Cog VMs available...)
On Fri, Jul 18, 2014 at 12:38 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
Stupid me. It's got to be the DeflateStream, and its subclasses ZipWriteStream, GZipWriteStream and ZLibWriteStream. So my change screws all uses of these in plugins. So what to do about fixing this? I would like to have a go at fixing the plugins. But cheaper might be to fix WriteStream with some property hack, storing the initialPosition somewhere else (a class side weak dictionary of stream -> initialPosition ?? (yuck)).
On Fri, Jul 18, 2014 at 12:31 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Thu, Jul 17, 2014 at 4:02 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 12:24:29AM +0200, Nicolas Cellier wrote:
2014-07-13 16:22 GMT+02:00 David T. Lewis lewis@mail.msen.com:
On Sun, Jul 13, 2014 at 09:55:41AM -0400, David T. Lewis wrote:
On Sun, Jul 06, 2014 at 12:47:25PM -0400, David T. Lewis wrote: > On Sun, Jul 06, 2014 at 12:23:11PM -0400, David T. Lewis wrote: > > On Sun, Jul 06, 2014 at 10:34:15AM +0200, Nicolas Cellier
wrote:
> > > 2014-07-04 0:47 GMT+02:00 Nicolas Cellier < > > > nicolas.cellier.aka.nice@gmail.com>: > > > > > > > > > > > > > > > > > > > 2014-07-04 0:09 GMT+02:00 Eliot Miranda <
eliot.miranda@gmail.com
: > > > > > > > > > > > >> > > > >> > > > >> On Thu, Jul 3, 2014 at 12:33 PM, Nicolas Cellier < > > > >> nicolas.cellier.aka.nice@gmail.com> wrote: > > > >> > > > >>> The bug is repeatable, i simply have to execute this
snippet
with my
> > > >>> test file: > > > >>> > > > >>> (FileStream fileNamed: 'snapshot.bin') binary
contentsOfEntireFile
> > > >>> asString zipped. > > > >>> > > > >>> The file is too big for an attachment here - 7.3 Mbytes
- or
1.7 Mbytes
> > > >>> gzipped by external tool. > > > >>> If someone can suggest an upload site, or want it by
mail,
just ask.
> > > >>> > > > >> > > > >> You're welcome to put it on ftp.mirandanamda.org,
cogftpuser,
pw cogging
> > > >> with 0's & 1's. > > > >> > > > >> > > > > done, pollution of your site engaged... > > > > Thanks Eliot! > > > > > > > > > > > >> > > > I inquired a bit more about this bug. > > > An important clue is that it does not happen in Pharo3.0! > > > > > > But Pharo3.0 did not fundamentally change compression > > > (except some FileSystem related changes, separation of CRC
stuff
in another
> > > package, some other cosmetic changes like replacing some self
assert: ...
> > > by [...] assert and a potential bug in > > > DeflateStream>>nextPutAll:startingAt: introduced by
CamilleTeruel).
> > > Squeak behavior is presumably not due to a Compression
change.
> > > > > > Neither is it a VM problem, the bugs still shows up when
running
the Squeak
> > > image with Pharo VM. > > > > > > > Yes, it is definite a problem in the Squeak trunk image. > > > > > > > So the difference lies somewhere else: in our beloved Stream. > > > I remembered a recent change of Eliot related to handling of
Stream created
> > > with on:from:to: (Collections-eem.567). > > > Reverting those changes just make the snippet pass! > > > > > > Ah ah! At the time, i found the change of Eliot quite
minimal and
nice.
> > > But I wish I had time to analyze this small innocent change
deeper.
> > > Indeed, I remembered I previously broke my teeth on this one
2
years or so
> > > ago, and preferred to adopt another strategy: avoid using
on:from:to: and
> > > ReadWriteStream as much as possible. > > > Why? because when analyzing the usage of inst.var. in Stream
hierarchy, it
> > > gave me headaches, some subclass are just ignoring superclass
variables, or
> > > worse are bending the semantics of superclass variables. > > > I came to the conclusion that I could not decently master a
change
of
> > > on:from:to: > > > > I can not confirm this. I have an image in which I can reliably
reproduce the
> > failure in writing the MCZ to a file. I tried reverting the
methods
that were
> > introduced in Collections-eem.567 and I am still getting the
same
failure.
> > > > Dave > > Oops, I spoke too soon. Indeed the problem appears as of
Collections-eem.567,
> and does not appear in Collections-nice.566. > > So I am confirming Nicolas' observations ... now to find the bug
:)
> > Dave >
This seems to be a rather tricky bug, and I don't think any of us
knows
the
cause right now. In case anyone else wants to have a look at it,
here is
a
recipe for reproducing the bug in a clean 4.5 image:
- Start with a clean Squeak 4.5 image in and empty working
directory.
ftp://ftp.squeak.org/4.5/Squeak4.5-13680.zip
Open the image, do not update it.
Open an MC browser and add the http://source.squeak.org/trunk
repository for the Help-Squeak-Project package and Collections
package.
Load Help-Squeak-Project-kfr.18 from trunk.
Load Collections-eem.567 from trunk.
Open a browser on class SqueakToolsDebuggerHelp, and delete the
class
side method #showDebuggerMenuForm.
- Remove the http://source.squeak.org/trunk repository form the
Help-Squeak-Project package in the MC browser.
- Highlight your package-cache repository and try to save
Help-Squeak-project to the package-cache repository.
Enter author initials 'dtl'.
For the package comment, enter 'Remove unreferenced method'.
Package save will fail part way through saving the MCZ.
To reproduce the failure in this image, do the following in a
workspace:
mcv := MCVersion allInstances detect: [:e | e name = 'aMCVersion(Help-Squeak-Project-dtl.19)'].
[f := FileStream fileNamed: 'junk.mcz'. mcv fileOutOn: f] ensure: [f close].In addition to the above recipe, here are some other observations:
- Reverting from Collections-eem.567 to Collections-nice.566 makes
the
problem go away, but ONLY if the DeflatePlugin is active.
- The failure appears while executing the fallback code for
#primitiveDeflateBlock in ZipWriteStream>>deflateBlock:chainLength:goodMatch:
- If this primitive is deactivate by commenting out the <primitive: 'primitiveDeflateBlock' module: 'ZipPlugin'> in ZipWriteStream>>deflateBlock:chainLength:goodMatch: then the
problem will
occur regardless of which version of Collections is loaded.
- When Collections-eem.567 is loaded, the primitive will fail on the
22nd
call to #deflateBlock:chainLength:goodMatch: and a debugger appears during execution of the fallback code.
- When Collections-nice.566 is loaded, the primitive does not fail
on the
22nd call to #deflateBlock:chainLength:goodMatch:, the fallback code is
never
run, and no error appears.
- When Collections-nice.566 is loaded and the primitive is disabled
to
force use of the fallback code, the error appears on on the 22nd call to #deflateBlock:chainLength:goodMatch: in exactlly the same place as
when
Collections-eem.567 is loaded.
- If I inspect the ZipWriteStream at the failure point (when the
debugger
pops up after the problem), I see no obvious difference in the state of
the
stream between the Collections-eem.567 case versus the
Collections-nice.566 with
primitive disabled case.
Dave
Very good mining work! When I hear Stream, I think fluid... But these snags make it so viscous, it's like our Stream is going to
freeze
soon. Or is it more than frozen? The vein you're after sounds as hard as
diamond,
we gonna need a sharp peak pickaxe.
I keep hoping that your stream fixes will somehow make this problem go away, but maybe we are not so lucky.
Whenever I get more time to play with this, I think I will try to catch it with gdb in the primitive. There is something strange happening that seems to first be detectable in the primitive failure, but only if Eliot's image fix is present.
This kind of problem is a nice way to spend the morning with a good cup of coffee, as some people do with the NY Times crossword puzzle ;)
http://en.wikipedia.org/wiki/The_New_York_Times_crossword_puzzle
Another "morning coffee" project is to figure out which version of the LargeIntegersPlugin should be used, which of course was the original topic that started this thread.
So given that the change adds an instance variable to WriteStream could that affect some plugin that somehow accesses a stream? What plugin could that be? I don't see any obvious inst vars in the DeflatePlugin or InflatePlugin. To remind ourselves, the three changes are
add inst var to WriteStream:
PositionableStream subclass: #WriteStream instanceVariableNames: 'writeLimit initialPositionOrNil' classVariableNames: '' poolDictionaries: '' category: 'Collections-Streams'
use it in two places: contents "Answer with a copy of my collection from the start to the current position."
readLimit := readLimit max: position. ^collection copyFrom: (initialPositionOrNil ifNil: [1]) to: position
on: aCollection from: firstIndex to: lastIndex
| len | collection := aCollection. readLimit := writeLimit := lastIndex > (len := collection size) ifTrue: [len] ifFalse: [lastIndex]. position := firstIndex <= 1 ifTrue: [0] ifFalse: [firstIndex - 1]. initialPositionOrNil := position + 1
What I've just found is that if I revert all three changes to WriteStream the bug goes away, but if I only revert the two method changes the bug remains. So I think the problem is merely the adding of the inst var. This could break some plugin which was expecting inst vars in subclasses of WriteStream to be at particular offsets determined by WriteStream instSize = 4 now being 5. The question is which plugin(s)?
Doh!
DeflatePlugin>>loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: 4 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: 5 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: 6 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: 7 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: 8 ofObject: rcvr." oop := interpreterProxy fetchPointer: 9 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 10 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 11 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 12 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: 13 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: 14 ofObject: rcvr.
^interpreterProxy failed not
I propose to add a variable that holds the inst size of the superclass of InflateStream, and to use this to correct the offsets. Objections?
On Fri, Jul 18, 2014 at 12:38 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
Stupid me. It's got to be the DeflateStream, and its subclasses ZipWriteStream, GZipWriteStream and ZLibWriteStream. So my change screws all uses of these in plugins. So what to do about fixing this? I would like to have a go at fixing the plugins. But cheaper might be to fix WriteStream with some property hack, storing the initialPosition somewhere else (a class side weak dictionary of stream -> initialPosition ?? (yuck)).
On Fri, Jul 18, 2014 at 12:31 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Thu, Jul 17, 2014 at 4:02 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 12:24:29AM +0200, Nicolas Cellier wrote:
2014-07-13 16:22 GMT+02:00 David T. Lewis lewis@mail.msen.com:
On Sun, Jul 13, 2014 at 09:55:41AM -0400, David T. Lewis wrote: > On Sun, Jul 06, 2014 at 12:47:25PM -0400, David T. Lewis wrote: > > On Sun, Jul 06, 2014 at 12:23:11PM -0400, David T. Lewis
wrote:
> > > On Sun, Jul 06, 2014 at 10:34:15AM +0200, Nicolas Cellier
wrote:
> > > > 2014-07-04 0:47 GMT+02:00 Nicolas Cellier < > > > > nicolas.cellier.aka.nice@gmail.com>: > > > > > > > > > > > > > > > > > > > > > > > > 2014-07-04 0:09 GMT+02:00 Eliot Miranda <
eliot.miranda@gmail.com
>: > > > > > > > > > > > > > > >> > > > > >> > > > > >> On Thu, Jul 3, 2014 at 12:33 PM, Nicolas Cellier < > > > > >> nicolas.cellier.aka.nice@gmail.com> wrote: > > > > >> > > > > >>> The bug is repeatable, i simply have to execute this
snippet
with my > > > > >>> test file: > > > > >>> > > > > >>> (FileStream fileNamed: 'snapshot.bin') binary contentsOfEntireFile > > > > >>> asString zipped. > > > > >>> > > > > >>> The file is too big for an attachment here - 7.3
Mbytes
- or
1.7 Mbytes > > > > >>> gzipped by external tool. > > > > >>> If someone can suggest an upload site, or want it by
mail,
just ask. > > > > >>> > > > > >> > > > > >> You're welcome to put it on ftp.mirandanamda.org,
cogftpuser,
pw cogging > > > > >> with 0's & 1's. > > > > >> > > > > >> > > > > > done, pollution of your site engaged... > > > > > Thanks Eliot! > > > > > > > > > > > > > > >> > > > > I inquired a bit more about this bug. > > > > An important clue is that it does not happen in Pharo3.0! > > > > > > > > But Pharo3.0 did not fundamentally change compression > > > > (except some FileSystem related changes, separation of CRC
stuff
in another > > > > package, some other cosmetic changes like replacing some
self
assert: ... > > > > by [...] assert and a potential bug in > > > > DeflateStream>>nextPutAll:startingAt: introduced by
CamilleTeruel).
> > > > Squeak behavior is presumably not due to a Compression
change.
> > > > > > > > Neither is it a VM problem, the bugs still shows up when
running
the Squeak > > > > image with Pharo VM. > > > > > > > > > > Yes, it is definite a problem in the Squeak trunk image. > > > > > > > > > > So the difference lies somewhere else: in our beloved
Stream.
> > > > I remembered a recent change of Eliot related to handling
of
Stream created > > > > with on:from:to: (Collections-eem.567). > > > > Reverting those changes just make the snippet pass! > > > > > > > > Ah ah! At the time, i found the change of Eliot quite
minimal and
nice. > > > > But I wish I had time to analyze this small innocent
change deeper.
> > > > Indeed, I remembered I previously broke my teeth on this
one 2
years or so > > > > ago, and preferred to adopt another strategy: avoid using on:from:to: and > > > > ReadWriteStream as much as possible. > > > > Why? because when analyzing the usage of inst.var. in
Stream
hierarchy, it > > > > gave me headaches, some subclass are just ignoring
superclass
variables, or > > > > worse are bending the semantics of superclass variables. > > > > I came to the conclusion that I could not decently master
a change
of > > > > on:from:to: > > > > > > I can not confirm this. I have an image in which I can
reliably
reproduce the > > > failure in writing the MCZ to a file. I tried reverting the
methods
that were > > > introduced in Collections-eem.567 and I am still getting the
same
failure. > > > > > > Dave > > > > Oops, I spoke too soon. Indeed the problem appears as of Collections-eem.567, > > and does not appear in Collections-nice.566. > > > > So I am confirming Nicolas' observations ... now to find the
bug :)
> > > > Dave > > > > This seems to be a rather tricky bug, and I don't think any of
us knows
the > cause right now. In case anyone else wants to have a look at it,
here is
a > recipe for reproducing the bug in a clean 4.5 image: > > - Start with a clean Squeak 4.5 image in and empty working
directory.
> ftp://ftp.squeak.org/4.5/Squeak4.5-13680.zip > > - Open the image, do not update it. > > - Open an MC browser and add the http://source.squeak.org/trunk repository for the Help-Squeak-Project package and Collections
package.
> > - Load Help-Squeak-Project-kfr.18 from trunk. > > - Load Collections-eem.567 from trunk. > > - Open a browser on class SqueakToolsDebuggerHelp, and delete
the class
side method #showDebuggerMenuForm. > > - Remove the http://source.squeak.org/trunk repository form the Help-Squeak-Project package in the MC browser. > > - Highlight your package-cache repository and try to save Help-Squeak-project to the package-cache repository. > > - Enter author initials 'dtl'. > > - For the package comment, enter 'Remove unreferenced method'. > > - Package save will fail part way through saving the MCZ. > > - To reproduce the failure in this image, do the following in a workspace: > > mcv := MCVersion allInstances detect: [:e | e name = 'a MCVersion(Help-Squeak-Project-dtl.19)']. > [f := FileStream fileNamed: 'junk.mcz'. > mcv fileOutOn: f] > ensure: [f close]. >
In addition to the above recipe, here are some other observations:
- Reverting from Collections-eem.567 to Collections-nice.566 makes
the
problem go away, but ONLY if the DeflatePlugin is active.
- The failure appears while executing the fallback code for
#primitiveDeflateBlock in ZipWriteStream>>deflateBlock:chainLength:goodMatch:
- If this primitive is deactivate by commenting out the <primitive: 'primitiveDeflateBlock' module: 'ZipPlugin'> in ZipWriteStream>>deflateBlock:chainLength:goodMatch: then the
problem will
occur regardless of which version of Collections is loaded.
- When Collections-eem.567 is loaded, the primitive will fail on
the 22nd
call to #deflateBlock:chainLength:goodMatch: and a debugger
appears
during execution of the fallback code.
- When Collections-nice.566 is loaded, the primitive does not fail
on the
22nd call to #deflateBlock:chainLength:goodMatch:, the fallback code
is never
run, and no error appears.
- When Collections-nice.566 is loaded and the primitive is
disabled to
force use of the fallback code, the error appears on on the 22nd call
to
#deflateBlock:chainLength:goodMatch: in exactlly the same place
as when
Collections-eem.567 is loaded.
- If I inspect the ZipWriteStream at the failure point (when the
debugger
pops up after the problem), I see no obvious difference in the state
of the
stream between the Collections-eem.567 case versus the
Collections-nice.566 with
primitive disabled case.
Dave
Very good mining work! When I hear Stream, I think fluid... But these snags make it so viscous, it's like our Stream is going to
freeze
soon. Or is it more than frozen? The vein you're after sounds as hard as
diamond,
we gonna need a sharp peak pickaxe.
I keep hoping that your stream fixes will somehow make this problem go away, but maybe we are not so lucky.
Whenever I get more time to play with this, I think I will try to catch it with gdb in the primitive. There is something strange happening that seems to first be detectable in the primitive failure, but only if Eliot's image fix is present.
This kind of problem is a nice way to spend the morning with a good cup of coffee, as some people do with the NY Times crossword puzzle ;)
http://en.wikipedia.org/wiki/The_New_York_Times_crossword_puzzle
Another "morning coffee" project is to figure out which version of the LargeIntegersPlugin should be used, which of course was the original topic that started this thread.
So given that the change adds an instance variable to WriteStream could that affect some plugin that somehow accesses a stream? What plugin could that be? I don't see any obvious inst vars in the DeflatePlugin or InflatePlugin. To remind ourselves, the three changes are
add inst var to WriteStream:
PositionableStream subclass: #WriteStream instanceVariableNames: 'writeLimit initialPositionOrNil' classVariableNames: '' poolDictionaries: '' category: 'Collections-Streams'
use it in two places: contents "Answer with a copy of my collection from the start to the current position."
readLimit := readLimit max: position. ^collection copyFrom: (initialPositionOrNil ifNil: [1]) to: position
on: aCollection from: firstIndex to: lastIndex
| len | collection := aCollection. readLimit := writeLimit := lastIndex > (len := collection size) ifTrue: [len] ifFalse: [lastIndex]. position := firstIndex <= 1 ifTrue: [0] ifFalse: [firstIndex - 1]. initialPositionOrNil := position + 1
What I've just found is that if I revert all three changes to WriteStream the bug goes away, but if I only revert the two method changes the bug remains. So I think the problem is merely the adding of the inst var. This could break some plugin which was expecting inst vars in subclasses of WriteStream to be at particular offsets determined by WriteStream instSize = 4 now being 5. The question is which plugin(s)?
Doh!
DeflatePlugin>>loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: 4 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: 5 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: 6 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: 7 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: 8 ofObject: rcvr." oop := interpreterProxy fetchPointer: 9 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 10 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 11 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 12 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: 13 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: 14 ofObject: rcvr.
^interpreterProxy failed not
I propose to add a variable that holds the inst size of the superclass of InflateStream, and to use this to correct the offsets. Objections? --
I can't look at the code right now, but any fix needs to be done in the image, otherwise all existing VMs are broken. Can we arrange for the added instance variable to be the last instance variable such that it does not effect existing references from the plugins (suitable commented of course)? Please disregard if I am misunderstanding, I don't have an image to look at.
Note that the primitive fallback code is probably still broken, but perhaps that's a separate issue entirely.
Dave
Hi David,
On Fri, Jul 18, 2014 at 1:51 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 12:38 PM, Eliot Miranda <
eliot.miranda@gmail.com>
wrote:
Stupid me. It's got to be the DeflateStream, and its subclasses ZipWriteStream, GZipWriteStream and ZLibWriteStream. So my change screws all uses of these in plugins. So what to do about fixing this? I would like to have a go at fixing the plugins. But cheaper might be to fix WriteStream with some property hack, storing the initialPosition somewhere else (a class side weak dictionary of stream -> initialPosition ?? (yuck)).
On Fri, Jul 18, 2014 at 12:31 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Thu, Jul 17, 2014 at 4:02 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 12:24:29AM +0200, Nicolas Cellier wrote:
2014-07-13 16:22 GMT+02:00 David T. Lewis lewis@mail.msen.com:
> On Sun, Jul 13, 2014 at 09:55:41AM -0400, David T. Lewis wrote: > > On Sun, Jul 06, 2014 at 12:47:25PM -0400, David T. Lewis wrote: > > > On Sun, Jul 06, 2014 at 12:23:11PM -0400, David T. Lewis
wrote:
> > > > On Sun, Jul 06, 2014 at 10:34:15AM +0200, Nicolas Cellier
wrote:
> > > > > 2014-07-04 0:47 GMT+02:00 Nicolas Cellier < > > > > > nicolas.cellier.aka.nice@gmail.com>: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2014-07-04 0:09 GMT+02:00 Eliot Miranda <
eliot.miranda@gmail.com
> >: > > > > > > > > > > > > > > > > > >> > > > > > >> > > > > > >> On Thu, Jul 3, 2014 at 12:33 PM, Nicolas Cellier < > > > > > >> nicolas.cellier.aka.nice@gmail.com> wrote: > > > > > >> > > > > > >>> The bug is repeatable, i simply have to execute this
snippet
> with my > > > > > >>> test file: > > > > > >>> > > > > > >>> (FileStream fileNamed: 'snapshot.bin') binary > contentsOfEntireFile > > > > > >>> asString zipped. > > > > > >>> > > > > > >>> The file is too big for an attachment here - 7.3
Mbytes
- or
> 1.7 Mbytes > > > > > >>> gzipped by external tool. > > > > > >>> If someone can suggest an upload site, or want it by
mail,
> just ask. > > > > > >>> > > > > > >> > > > > > >> You're welcome to put it on ftp.mirandanamda.org,
cogftpuser,
> pw cogging > > > > > >> with 0's & 1's. > > > > > >> > > > > > >> > > > > > > done, pollution of your site engaged... > > > > > > Thanks Eliot! > > > > > > > > > > > > > > > > > >> > > > > > I inquired a bit more about this bug. > > > > > An important clue is that it does not happen in Pharo3.0! > > > > > > > > > > But Pharo3.0 did not fundamentally change compression > > > > > (except some FileSystem related changes, separation of CRC
stuff
> in another > > > > > package, some other cosmetic changes like replacing some
self
> assert: ... > > > > > by [...] assert and a potential bug in > > > > > DeflateStream>>nextPutAll:startingAt: introduced by
CamilleTeruel).
> > > > > Squeak behavior is presumably not due to a Compression
change.
> > > > > > > > > > Neither is it a VM problem, the bugs still shows up when
running
> the Squeak > > > > > image with Pharo VM. > > > > > > > > > > > > > Yes, it is definite a problem in the Squeak trunk image. > > > > > > > > > > > > > So the difference lies somewhere else: in our beloved
Stream.
> > > > > I remembered a recent change of Eliot related to handling
of
> Stream created > > > > > with on:from:to: (Collections-eem.567). > > > > > Reverting those changes just make the snippet pass! > > > > > > > > > > Ah ah! At the time, i found the change of Eliot quite
minimal and
> nice. > > > > > But I wish I had time to analyze this small innocent
change deeper.
> > > > > Indeed, I remembered I previously broke my teeth on this
one 2
> years or so > > > > > ago, and preferred to adopt another strategy: avoid using > on:from:to: and > > > > > ReadWriteStream as much as possible. > > > > > Why? because when analyzing the usage of inst.var. in
Stream
> hierarchy, it > > > > > gave me headaches, some subclass are just ignoring
superclass
> variables, or > > > > > worse are bending the semantics of superclass variables. > > > > > I came to the conclusion that I could not decently master
a change
> of > > > > > on:from:to: > > > > > > > > I can not confirm this. I have an image in which I can
reliably
> reproduce the > > > > failure in writing the MCZ to a file. I tried reverting the
methods
> that were > > > > introduced in Collections-eem.567 and I am still getting the
same
> failure. > > > > > > > > Dave > > > > > > Oops, I spoke too soon. Indeed the problem appears as of > Collections-eem.567, > > > and does not appear in Collections-nice.566. > > > > > > So I am confirming Nicolas' observations ... now to find the
bug :)
> > > > > > Dave > > > > > > > This seems to be a rather tricky bug, and I don't think any of
us knows
> the > > cause right now. In case anyone else wants to have a look at it,
here is
> a > > recipe for reproducing the bug in a clean 4.5 image: > > > > - Start with a clean Squeak 4.5 image in and empty working
directory.
> > ftp://ftp.squeak.org/4.5/Squeak4.5-13680.zip > > > > - Open the image, do not update it. > > > > - Open an MC browser and add the http://source.squeak.org/trunk > repository for the Help-Squeak-Project package and Collections
package.
> > > > - Load Help-Squeak-Project-kfr.18 from trunk. > > > > - Load Collections-eem.567 from trunk. > > > > - Open a browser on class SqueakToolsDebuggerHelp, and delete
the class
> side method #showDebuggerMenuForm. > > > > - Remove the http://source.squeak.org/trunk repository form the > Help-Squeak-Project package in the MC browser. > > > > - Highlight your package-cache repository and try to save > Help-Squeak-project to the package-cache repository. > > > > - Enter author initials 'dtl'. > > > > - For the package comment, enter 'Remove unreferenced method'. > > > > - Package save will fail part way through saving the MCZ. > > > > - To reproduce the failure in this image, do the following in a > workspace: > > > > mcv := MCVersion allInstances detect: [:e | e name = 'a > MCVersion(Help-Squeak-Project-dtl.19)']. > > [f := FileStream fileNamed: 'junk.mcz'. > > mcv fileOutOn: f] > > ensure: [f close]. > > > > In addition to the above recipe, here are some other observations: > > - Reverting from Collections-eem.567 to Collections-nice.566 makes
the
> problem go away, but ONLY if the DeflatePlugin is active. > > - The failure appears while executing the fallback code for > #primitiveDeflateBlock > in ZipWriteStream>>deflateBlock:chainLength:goodMatch: > > - If this primitive is deactivate by commenting out the > <primitive: 'primitiveDeflateBlock' module: 'ZipPlugin'> in > ZipWriteStream>>deflateBlock:chainLength:goodMatch: then the
problem will
> occur regardless of which version of Collections is loaded. > > - When Collections-eem.567 is loaded, the primitive will fail on
the 22nd
> call to #deflateBlock:chainLength:goodMatch: and a debugger
appears
> during > execution of the fallback code. > > - When Collections-nice.566 is loaded, the primitive does not fail
on the
> 22nd > call to #deflateBlock:chainLength:goodMatch:, the fallback code
is never
> run, > and no error appears. > > - When Collections-nice.566 is loaded and the primitive is
disabled to
> force > use of the fallback code, the error appears on on the 22nd call
to
> #deflateBlock:chainLength:goodMatch: in exactlly the same place
as when
> Collections-eem.567 is loaded. > > - If I inspect the ZipWriteStream at the failure point (when the
debugger
> pops > up after the problem), I see no obvious difference in the state
of the
> stream > between the Collections-eem.567 case versus the
Collections-nice.566 with
> primitive disabled case. > > Dave > > Very good mining work! When I hear Stream, I think fluid... But these snags make it so viscous, it's like our Stream is going to
freeze
soon. Or is it more than frozen? The vein you're after sounds as hard as
diamond,
we gonna need a sharp peak pickaxe.
I keep hoping that your stream fixes will somehow make this problem go away, but maybe we are not so lucky.
Whenever I get more time to play with this, I think I will try to catch it with gdb in the primitive. There is something strange happening that seems to first be detectable in the primitive failure, but only if Eliot's image fix is present.
This kind of problem is a nice way to spend the morning with a good cup of coffee, as some people do with the NY Times crossword puzzle ;)
http://en.wikipedia.org/wiki/The_New_York_Times_crossword_puzzle
Another "morning coffee" project is to figure out which version of the LargeIntegersPlugin should be used, which of course was the original topic that started this thread.
So given that the change adds an instance variable to WriteStream could that affect some plugin that somehow accesses a stream? What plugin could that be? I don't see any obvious inst vars in the DeflatePlugin or InflatePlugin. To remind ourselves, the three changes are
add inst var to WriteStream:
PositionableStream subclass: #WriteStream instanceVariableNames: 'writeLimit initialPositionOrNil' classVariableNames: '' poolDictionaries: '' category: 'Collections-Streams'
use it in two places: contents "Answer with a copy of my collection from the start to the current position."
readLimit := readLimit max: position. ^collection copyFrom: (initialPositionOrNil ifNil: [1]) to: position
on: aCollection from: firstIndex to: lastIndex
| len | collection := aCollection. readLimit := writeLimit := lastIndex > (len := collection size) ifTrue: [len] ifFalse: [lastIndex]. position := firstIndex <= 1 ifTrue: [0] ifFalse: [firstIndex - 1]. initialPositionOrNil := position + 1
What I've just found is that if I revert all three changes to WriteStream the bug goes away, but if I only revert the two method changes the bug remains. So I think the problem is merely the adding of the inst var. This could break some plugin which was expecting inst vars in subclasses of WriteStream to be at particular offsets determined by WriteStream instSize = 4 now being 5. The question is which plugin(s)?
Doh!
DeflatePlugin>>loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: 4 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: 5 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: 6 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: 7 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: 8 ofObject: rcvr." oop := interpreterProxy fetchPointer: 9 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 10 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 11 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 12 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: 13 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: 14 ofObject: rcvr.
^interpreterProxy failed not
I propose to add a variable that holds the inst size of the superclass of InflateStream, and to use this to correct the offsets. Objections? --
I can't look at the code right now, but any fix needs to be done in the image, otherwise all existing VMs are broken.
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin & DeflatePlugin)'s dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the restriction long-term.
Can we arrange for the added instance variable to be the last instance variable such that it does not effect existing references from the plugins (suitable commented of course)? Please disregard if I am misunderstanding, I don't have an image to look at.
The issue is that the inst var added (initialPositionOrNil) is to ReadStream and WriteStream and hence affects the inst var offsets in all subclasses, and these include DeflateStream, ZipEncoder and InflateStream, which the ZipPlugin accesses.
Note that the primitive fallback code is probably still broken, but
perhaps that's a separate issue entirely.
I'm not convinced yet. Certainly if the primitives are all removed and the image still fails then the fall;back code is broken. But we need to determine that carefully.
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin & DeflatePlugin)'s dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the restriction long-term.
Is this fix is of sufficient urgency to justify asking all Squeak trunk users to replace their VMs? Is there any less disruptive way to address the issue? I don't know the answers, just asking.
Dave
On Fri, Jul 18, 2014 at 4:20 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin &
DeflatePlugin)'s
dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the
restriction
long-term.
Is this fix is of sufficient urgency to justify asking all Squeak trunk users to replace their VMs? Is there any less disruptive way to address the issue? I don't know the answers, just asking.
Well, I've fixed the VM (commit to appear shortly) and much prefer having a fixed plugin. The alternatives are a) having some horrible hack in the image to store initialPositionOrNil out of streams b) living with on:from:to: being broken
Since VMs can be fixed it seems to me that the best solution is to build new VMs. That's the solution I'm following anyway.
The *plugin* is fixed and can be provided as a fix for current vms. No need to require entire new vms. It's what the plugin idea was invented for... So long as we haven't broken the functionality since '99 or whenever it was that Andreas & I added it.
Looks like we have several avenues for fixes; an image update that removes the problematic change pro tem A plugin solus patch that makes the image as-is Just Work Maybe the plugin and some image code to detect it, use it if possible, alternative st code otherwise? And of course, replacement vms.
Hopefully it is not beyond our collective wit to make an update that fetches the correct plugin and installs it.
/tim {insert witticism here}
On Fri, Jul 18, 2014 at 6:42 PM, tim Rowledge tim@rowledge.org wrote:
The *plugin* is fixed and can be provided as a fix for current vms. No need to require entire new vms. It's what the plugin idea was invented for... So long as we haven't broken the functionality since '99 or whenever it was that Andreas & I added it.
Looks like we have several avenues for fixes; an image update that removes the problematic change pro tem A plugin solus patch that makes the image as-is Just Work Maybe the plugin and some image code to detect it, use it if possible, alternative st code otherwise? And of course, replacement vms.
Hopefully it is not beyond our collective wit to make an update that fetches the correct plugin and installs it.
It is internal in the Cog VMs. And before you condemn that choice it /is/ slightly more efficient as it doesn't have to indirect to access the interpreterProxy API. But in any case new Cog VMs are already available.
/tim {insert witticism here}
It is internal in the Cog VMs. And before you condemn that choice it /is/ slightly more efficient as it doesn't have to indirect to access the interpreterProxy API. But in any case new Cog VMs are already available.
I know - your more direct macro access is a good idea. But that wasnt the point I intended; unless we've broken the plugin mechanism over the years a fixed-up zip plugin will over-ride the broken internal code, offering a very easy fix. It should even work in a running system where you don't want to quit and restart with a new vm for whatever reason. Something like Smalltalk unloadModule: 'ziplthingy' Should disconnect from the old version and the next call to anything in the plugin ought to load the new external copy. Obviously this could be complicated by any stored state, so caveat emptor.
It's a really neat trick when developing a plugin.
On Sat, Jul 19, 2014 at 09:50:42AM -0700, tim Rowledge wrote:
It is internal in the Cog VMs. And before you condemn that choice it /is/ slightly more efficient as it doesn't have to indirect to access the interpreterProxy API. But in any case new Cog VMs are already available.
I know - your more direct macro access is a good idea. But that wasnt the point I intended; unless we've broken the plugin mechanism over the years a fixed-up zip plugin will over-ride the broken internal code, offering a very easy fix. It should even work in a running system where you don't want to quit and restart with a new vm for whatever reason. Something like Smalltalk unloadModule: 'ziplthingy' Should disconnect from the old version and the next call to anything in the plugin ought to load the new external copy. Obviously this could be complicated by any stored state, so caveat emptor.
It's a really neat trick when developing a plugin.
Yes, this mechanism does work, and it is quite feasible to distribute an external plugin to override an existing internal plugin. You are right that is it not necessary to distribute a new VM. From the point of view of a Squeak user who may not be familiar with how the VM is put together, it may be something of an academic point since we have no extablished process for distributing plugin updates independent of the full VM installations.
Dave
On Jul 19, 2014, at 12:46, "David T. Lewis" lewis@mail.msen.com wrote:
It's a really neat trick when developing a plugin.
Yes, this mechanism does work, and it is quite feasible to distribute an external plugin to override an existing internal plugin. You are right that is it not necessary to distribute a new VM. From the point of view of a Squeak user who may not be familiar with how the VM is put together, it may be something of an academic point since we have no extablished process for distributing plugin updates independent of the full VM installations.
An excellent point. It was part of the original plan, as a way to provide bug fixes as well as flexible extensions to the vm. I seem to recall some worries about the ballooning size of the vm.... Technology has moved on since then though and even a fully bloated vm with every plugin ever written built internally is probably a small fraction of the size of typical email with yet another inline attached damn cat video.
On Sat, Jul 19, 2014 at 9:50 AM, tim Rowledge tim@rowledge.org wrote:
It is internal in the Cog VMs. And before you condemn that choice it /is/ slightly more efficient as it doesn't have to indirect to access the interpreterProxy API. But in any case new Cog VMs are already available.
I know - your more direct macro access is a good idea. But that wasnt the point I intended; unless we've broken the plugin mechanism over the years a fixed-up zip plugin will over-ride the broken internal code, offering a very easy fix.
Oh *cool*! So the loader prefers shared objects/dlls over internals if available? Great idea. However, it does seem to me that building and deploying a new VM is about the same complexity as building and deploying a new plugin. In fact, slightly less, cuz installing the plugin in an existing VM is, for some, tricky.
It should even work in a running system where you don't want to quit and restart with a new vm for whatever reason. Something like Smalltalk unloadModule: 'ziplthingy' Should disconnect from the old version and the next call to anything in the plugin ought to load the new external copy. Obviously this could be complicated by any stored state, so caveat emptor.
It's a really neat trick when developing a plugin.
Yes indeed.
On Jul 19, 2014, at 20:52, Eliot Miranda eliot.miranda@gmail.com wrote:
However, it does seem to me that building and deploying a new VM is about the same complexity as building and deploying a new plugin. In fact, slightly less, cuz installing the plugin in an existing VM is, for some, tricky.
Yah. We should fix that. Like about fifteen years ago.
/tim {insert witticism here}
Hey Tim,
... /tim {insert witticism here}
This seems to be broken. I miss the witticism:((
Lou ----------------------------------------------------------- Louis LaBrunda Keystone Software Corp. SkypeMe callto://PhotonDemon mailto:Lou@Keystone-Software.com http://www.Keystone-Software.com
On Jul 22, 2014, at 6:44, Louis LaBrunda Lou@Keystone-Software.com wrote:
/tim {insert witticism here}
This seems to be broken. I miss the witticism:((
My iMac is currently in hospital, suffering from some sort of nervous breakdown. Sadly iPad email doesn't do the random silliness trick, so until some cure is effected I'm afraid the service will be offline. Normal service will be resumed as soon as possible. In the meantime, please return to your seats and await fresh shipments of lemon scented paper napkins.
On 19.07.2014, at 02:15, Eliot Miranda eliot.miranda@gmail.com wrote:
On Fri, Jul 18, 2014 at 4:20 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin & DeflatePlugin)'s dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the restriction long-term.
Is this fix is of sufficient urgency to justify asking all Squeak trunk users to replace their VMs? Is there any less disruptive way to address the issue? I don't know the answers, just asking.
Well, I've fixed the VM (commit to appear shortly) and much prefer having a fixed plugin. The alternatives are a) having some horrible hack in the image to store initialPositionOrNil out of streams b) living with on:from:to: being broken
Since VMs can be fixed it seems to me that the best solution is to build new VMs. That's the solution I'm following anyway.
But ... breaking old images is *not* a good idea.
IMHO we should name the fixed plugin ZipPlugin2. The new image with changed ivar offsets will use that plugin. Old images with the original object layout will use the original.
- Bert -
On Mon, Jul 21, 2014 at 2:32 AM, Bert Freudenberg bert@freudenbergs.de wrote:
On 19.07.2014, at 02:15, Eliot Miranda eliot.miranda@gmail.com wrote:
On Fri, Jul 18, 2014 at 4:20 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin &
DeflatePlugin)'s
dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the
restriction
long-term.
Is this fix is of sufficient urgency to justify asking all Squeak trunk users to replace their VMs? Is there any less disruptive way to address the issue? I don't know the answers, just asking.
Well, I've fixed the VM (commit to appear shortly) and much prefer having a fixed plugin. The alternatives are a) having some horrible hack in the image to store initialPositionOrNil out of streams b) living with on:from:to: being broken
Since VMs can be fixed it seems to me that the best solution is to build new VMs. That's the solution I'm following anyway.
But ... breaking old images is *not* a good idea.
IMHO we should name the fixed plugin ZipPlugin2. The new image with changed ivar offsets will use that plugin. Old images with the original object layout will use the original.
But old images will continue to work with either plugin. And any image can back out of the fix to on:from:to: and delete the added inst vars and continue to work. What breaks things is adding an inst var to WriteStream or ReadStream, /not/ changing the plugin.
On 21.07.2014, at 11:47, Eliot Miranda eliot.miranda@gmail.com wrote:
On Mon, Jul 21, 2014 at 2:32 AM, Bert Freudenberg bert@freudenbergs.de wrote:
On 19.07.2014, at 02:15, Eliot Miranda eliot.miranda@gmail.com wrote:
On Fri, Jul 18, 2014 at 4:20 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin & DeflatePlugin)'s dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the restriction long-term.
Is this fix is of sufficient urgency to justify asking all Squeak trunk users to replace their VMs? Is there any less disruptive way to address the issue? I don't know the answers, just asking.
Well, I've fixed the VM (commit to appear shortly) and much prefer having a fixed plugin. The alternatives are a) having some horrible hack in the image to store initialPositionOrNil out of streams b) living with on:from:to: being broken
Since VMs can be fixed it seems to me that the best solution is to build new VMs. That's the solution I'm following anyway.
But ... breaking old images is *not* a good idea.
IMHO we should name the fixed plugin ZipPlugin2. The new image with changed ivar offsets will use that plugin. Old images with the original object layout will use the original.
But old images will continue to work with either plugin. And any image can back out of the fix to on:from:to: and delete the added inst vars and continue to work. What breaks things is adding an inst var to WriteStream or ReadStream, /not/ changing the plugin.
Ah, I only saw your inst-var-detection message now. That's cool :)
- Bert -
On Mon, Jul 21, 2014 at 02:47:23AM -0700, Eliot Miranda wrote:
On Mon, Jul 21, 2014 at 2:32 AM, Bert Freudenberg bert@freudenbergs.de wrote:
IMHO we should name the fixed plugin ZipPlugin2. The new image with changed ivar offsets will use that plugin. Old images with the original object layout will use the original.
But old images will continue to work with either plugin. And any image can back out of the fix to on:from:to: and delete the added inst vars and continue to work. What breaks things is adding an inst var to WriteStream or ReadStream, /not/ changing the plugin.
Exactly.
And just to make things more interesting, we've also found that there is an issue in the Smalltalk zip code that is exposed whenever the plugin is not working. So there were quite a few tails on this problem!
Dave
On Mon, Jul 21, 2014 at 11:32:42AM +0200, Bert Freudenberg wrote:
On 19.07.2014, at 02:15, Eliot Miranda eliot.miranda@gmail.com wrote:
On Fri, Jul 18, 2014 at 4:20 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
I think we have to accept that the fix to on:from:to: means that exsting VMs are broken. Certainly the ZipPlugin (InflatePlugin & DeflatePlugin)'s dependency on WriteStream and ReadStream inst size is restrictive. I'd rather lift the restriction and use new VMs than live with the restriction long-term.
Is this fix is of sufficient urgency to justify asking all Squeak trunk users to replace their VMs? Is there any less disruptive way to address the issue? I don't know the answers, just asking.
Well, I've fixed the VM (commit to appear shortly) and much prefer having a fixed plugin. The alternatives are a) having some horrible hack in the image to store initialPositionOrNil out of streams b) living with on:from:to: being broken
Since VMs can be fixed it seems to me that the best solution is to build new VMs. That's the solution I'm following anyway.
But ... breaking old images is *not* a good idea.
Right. That was the point of my original question. Maybe this belongs on squeak-dev rather than vm-dev, but what I meant to ask was:
1) In Collections-eem.567 we have a fix for ([Read|Write]Stream on:...from:...to:...) contents. Is there any other way to address that problem that does *not* add an instance variable that affects the plugins? (probably the answer is no, but it's worth asking)
2) Given that the fix in Collections-eem.567 causes problems both in the image and in the plugins, does that change carry it's weight? What practical problem does it solve, and is solving that problem worth the various side effects?
IMHO we should name the fixed plugin ZipPlugin2. The new image with changed ivar offsets will use that plugin. Old images with the original object layout will use the original.
The plugin changes are backward compatible, so it is not necessary to rename the plugin.
Dave
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
On Fri, Jul 18, 2014 at 1:51 PM, David T. Lewis lewis@mail.msen.com wrote:
Note that the primitive fallback code is probably still broken, but perhaps that's a separate issue entirely.
I'm not convinced yet. Certainly if the primitives are all removed and the image still fails then the fall;back code is broken. But we need to determine that carefully.
To follow up on this WRT the Smalltalk that runs if the primitive fails or if the plugin is not present:
I made a VM with no ZipPlugin, and tested with this. The failure occurs exactly as before. So the presence of a working ZipPlugin is masking an issue in the image. The issue appears if the plugin is absent, and it appears if the primitive fails due the the ivar issue that we have been discussing. The problem is hidden when a working ZipPlugin is present.
Sorry I still can't offer a test case, other than the "how to make it fail recipe" previously discussed.
Dave
On Sun, Jul 20, 2014 at 7:30 AM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
On Fri, Jul 18, 2014 at 1:51 PM, David T. Lewis lewis@mail.msen.com
wrote:
Note that the primitive fallback code is probably still broken, but perhaps that's a separate issue entirely.
I'm not convinced yet. Certainly if the primitives are all removed and
the
image still fails then the fall;back code is broken. But we need to determine that carefully.
To follow up on this WRT the Smalltalk that runs if the primitive fails or if the plugin is not present:
I made a VM with no ZipPlugin, and tested with this. The failure occurs exactly as before. So the presence of a working ZipPlugin is masking an issue in the image. The issue appears if the plugin is absent, and it appears if the primitive fails due the the ivar issue that we have been discussing. The problem is hidden when a working ZipPlugin is present.
This explains the appearance of the bug in the first place. Adding the inst vars to ReadStream and WriteStream to fix on:from:to: caused the plugin methods to fail, since values fetched from the streams were no longer as expected due to their offsets changing. This caused the back-up code to run.
Sorry I still can't offer a test case, other than the "how to make it fail
recipe" previously discussed.
I have a suspicion. If the plugin attempts to store a "byte" outside the 0-255 range, the least significant 8 bits get stored. If the image attempts to store a "byte" outside the 0-255 range, the target morphs to a WideString, which will change the offsets of lots of things.
On Sun, Jul 20, 2014 at 11:18:18AM -0700, Eliot Miranda wrote:
On Sun, Jul 20, 2014 at 7:30 AM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
On Fri, Jul 18, 2014 at 1:51 PM, David T. Lewis lewis@mail.msen.com
wrote:
Note that the primitive fallback code is probably still broken, but perhaps that's a separate issue entirely.
I'm not convinced yet. Certainly if the primitives are all removed and
the
image still fails then the fall;back code is broken. But we need to determine that carefully.
To follow up on this WRT the Smalltalk that runs if the primitive fails or if the plugin is not present:
I made a VM with no ZipPlugin, and tested with this. The failure occurs exactly as before. So the presence of a working ZipPlugin is masking an issue in the image. The issue appears if the plugin is absent, and it appears if the primitive fails due the the ivar issue that we have been discussing. The problem is hidden when a working ZipPlugin is present.
This explains the appearance of the bug in the first place. Adding the inst vars to ReadStream and WriteStream to fix on:from:to: caused the plugin methods to fail, since values fetched from the streams were no longer as expected due to their offsets changing. This caused the back-up code to run.
Yes, that is exactly what happened.
Sorry I still can't offer a test case, other than the "how to make it fail
recipe" previously discussed.
I have a suspicion. If the plugin attempts to store a "byte" outside the 0-255 range, the least significant 8 bits get stored. If the image attempts to store a "byte" outside the 0-255 range, the target morphs to a WideString, which will change the offsets of lots of things.
That may very well be the case! And this would explain why zip stream code that has lived happily in the image for a long time now seems to have become buggy.
Dave
On 20.07.2014, at 20:42, David T. Lewis lewis@mail.msen.com wrote:
On Sun, Jul 20, 2014 at 11:18:18AM -0700, Eliot Miranda wrote:
On Sun, Jul 20, 2014 at 7:30 AM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 02:06:18PM -0700, Eliot Miranda wrote:
On Fri, Jul 18, 2014 at 1:51 PM, David T. Lewis lewis@mail.msen.com
wrote:
Note that the primitive fallback code is probably still broken, but perhaps that's a separate issue entirely.
I'm not convinced yet. Certainly if the primitives are all removed and
the
image still fails then the fall;back code is broken. But we need to determine that carefully.
To follow up on this WRT the Smalltalk that runs if the primitive fails or if the plugin is not present:
I made a VM with no ZipPlugin, and tested with this. The failure occurs exactly as before. So the presence of a working ZipPlugin is masking an issue in the image. The issue appears if the plugin is absent, and it appears if the primitive fails due the the ivar issue that we have been discussing. The problem is hidden when a working ZipPlugin is present.
This explains the appearance of the bug in the first place. Adding the inst vars to ReadStream and WriteStream to fix on:from:to: caused the plugin methods to fail, since values fetched from the streams were no longer as expected due to their offsets changing. This caused the back-up code to run.
Yes, that is exactly what happened.
Sorry I still can't offer a test case, other than the "how to make it fail
recipe" previously discussed.
I have a suspicion. If the plugin attempts to store a "byte" outside the 0-255 range, the least significant 8 bits get stored. If the image attempts to store a "byte" outside the 0-255 range, the target morphs to a WideString, which will change the offsets of lots of things.
That may very well be the case! And this would explain why zip stream code that has lived happily in the image for a long time now seems to have become buggy.
Dave
- Bert -
Hi All,
I'm close to a fix on this, but I have to stop as I've some errands to run. I wonder if anyone is interested in reviewing the code. There's a mistake in there somewhere. Careful review will help.
The issue is to match the inst var offsets in the InflatePlugin and DeflatePlugin methods with those in InflateStream DeflateStream ZipWriteStream and ZipEncoder now that the plugin code uses readStreamInstSize and writeStreamInstSize to get at them. Perhaps some kind soul can read this code like a lawyer and spot my mistake? I'll try and take another look at this this evening.
Changes attached and included for your perusal:
'From Squeak4.5 of 18 July 2014 [latest update: #13848] on 18 July 2014 at 2:34:04 pm'! InflatePlugin subclass: #DeflatePlugin instanceVariableNames: 'zipHashHead zipHashTail zipHashValue zipBlockPos zipBlockStart zipLiterals zipDistances zipLiteralFreq zipDistanceFreq zipLiteralCount zipLiteralSize zipMatchCount zipMatchLengthCodes zipDistanceCodes zipCrcTable zipExtraLengthBits zipExtraDistanceBits zipBaseLength zipBaseDistance writeStreamInstSize' classVariableNames: 'DeflateHashBits DeflateHashMask DeflateHashShift DeflateHashTableSize DeflateMaxDistance DeflateMaxDistanceCodes DeflateMaxLiteralCodes DeflateMaxMatch DeflateMinMatch DeflateWindowMask DeflateWindowSize' poolDictionaries: '' category: 'VMMaker-Plugins'! !DeflatePlugin commentStamp: 'tpr 5/5/2003 11:52' prior: 0! This adds Zip deflating support. InflatePlugin should not be translated but this subclass should since it is incorporated within that class's translation process!
!DeflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 13:56'! determineSizeOfWriteStream: rcvr "Determine the inst size of the class above DeflateStream or ZipEncoder by looking for the first class whose inst size is less than 7." | class | class := interpreterProxy fetchClassOf: rcvr. [class ~= interpreterProxy nilObject and: [(interpreterProxy instanceSizeOf: class) >= 7]] whileTrue: [class := interpreterProxy superclassOf: class]. class = interpreterProxy nilObject ifTrue: [^false]. writeStreamInstSize := interpreterProxy instanceSizeOf: class. ^true ! !
!DeflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 14:15'! loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. writeStreamInstSize = 0 ifTrue: [(self determineSizeOfWriteStream: rcvr) ifFalse: [^false]. "If the receiver wasn't valid then we derived writeStreamInstSize from an invalid source. discard it." (interpreterProxy slotSizeOf: rcvr) < (writeStreamInstSize + 5) ifTrue: [writeStreamInstSize := 0. ^false]]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: writeStreamInstSize + 0 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: writeStreamInstSize + 1 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: writeStreamInstSize + 2 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: writeStreamInstSize + 3 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: writeStreamInstSize + 4 ofObject: rcvr." oop := interpreterProxy fetchPointer: writeStreamInstSize + 5 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: writeStreamInstSize + 6 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: writeStreamInstSize + 7 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: writeStreamInstSize + 8 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: writeStreamInstSize + 9 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: writeStreamInstSize + 10 ofObject: rcvr.
^interpreterProxy failed not! !
!DeflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 14:15'! loadZipEncoderFrom: rcvr | oop | <inline: false> writeStreamInstSize = 0 ifTrue: [(self determineSizeOfWriteStream: rcvr) ifFalse: [^false]. "If the receiver wasn't valid then we derived writeStreamInstSize from an invalid source. discard it." (interpreterProxy slotSizeOf: rcvr) < (writeStreamInstSize + 3) ifTrue: [writeStreamInstSize := 0. ^false]]. ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= (writeStreamInstSize + 3)]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^interpreterProxy primitiveFail]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr." zipBitBuf := interpreterProxy fetchInteger: writeStreamInstSize + 0 ofObject: rcvr. zipBitPos := interpreterProxy fetchInteger: writeStreamInstSize + 1 ofObject: rcvr.
^interpreterProxy failed not! !
!DeflatePlugin methodsFor: 'primitives' stamp: 'eem 7/18/2014 14:28'! primitiveDeflateBlock "Primitive. Deflate the current contents of the receiver." | goodMatch chainLength lastIndex rcvr result | <export: true> <inline: false> interpreterProxy methodArgumentCount = 3 ifFalse:[^interpreterProxy primitiveFail]. goodMatch := interpreterProxy stackIntegerValue: 0. chainLength := interpreterProxy stackIntegerValue: 1. lastIndex := interpreterProxy stackIntegerValue: 2. rcvr := interpreterProxy stackObjectValue: 3. interpreterProxy failed ifTrue:[^nil]. self cCode:'' inSmalltalk:[ zipMatchLengthCodes := CArrayAccessor on: ZipWriteStream matchLengthCodes. zipDistanceCodes := CArrayAccessor on: ZipWriteStream distanceCodes]. (self loadDeflateStreamFrom: rcvr) ifFalse:[^interpreterProxy primitiveFail]. result := self deflateBlock: lastIndex chainLength: chainLength goodMatch: goodMatch. interpreterProxy failed ifFalse:[ "Store back modified values" interpreterProxy storeInteger: writeStreamInstSize + 2 ofObject: rcvr withValue: zipHashValue. interpreterProxy storeInteger: writeStreamInstSize + 3 ofObject: rcvr withValue: zipBlockPos. interpreterProxy storeInteger: writeStreamInstSize + 9 ofObject: rcvr withValue: zipLiteralCount. interpreterProxy storeInteger: writeStreamInstSize + 10 ofObject: rcvr withValue: zipMatchCount]. interpreterProxy failed ifFalse:[ interpreterProxy pop: 4. interpreterProxy pushBool: result. ].! !
!DeflatePlugin methodsFor: 'primitives' stamp: 'eem 7/18/2014 14:24'! primitiveZipSendBlock | distTree litTree distStream litStream rcvr result | <export: true> interpreterProxy methodArgumentCount = 4 ifFalse:[^interpreterProxy primitiveFail]. distTree := interpreterProxy stackObjectValue: 0. litTree := interpreterProxy stackObjectValue: 1. distStream := interpreterProxy stackObjectValue: 2. litStream := interpreterProxy stackObjectValue: 3. rcvr := interpreterProxy stackObjectValue: 4. interpreterProxy failed ifTrue:[^nil]. (self loadZipEncoderFrom: rcvr) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: distTree) and:[ (interpreterProxy slotSizeOf: distTree) >= 2]) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: litTree) and:[ (interpreterProxy slotSizeOf: litTree) >= 2]) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: litStream) and:[ (interpreterProxy slotSizeOf: litStream) >= 3]) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: distStream) and:[ (interpreterProxy slotSizeOf: distStream) >= 3]) ifFalse:[^interpreterProxy primitiveFail]. self cCode:'' inSmalltalk:[ zipMatchLengthCodes := CArrayAccessor on: ZipWriteStream matchLengthCodes. zipDistanceCodes := CArrayAccessor on: ZipWriteStream distanceCodes. zipExtraLengthBits := CArrayAccessor on: ZipWriteStream extraLengthBits. zipExtraDistanceBits := CArrayAccessor on: ZipWriteStream extraDistanceBits. zipBaseLength := CArrayAccessor on: ZipWriteStream baseLength. zipBaseDistance := CArrayAccessor on: ZipWriteStream baseDistance]. result := self sendBlock: litStream with: distStream with: litTree with: distTree. interpreterProxy failed ifFalse:[ interpreterProxy storeInteger: 1 ofObject: rcvr withValue: zipPosition. interpreterProxy storeInteger: readStreamInstSize + 1 ofObject: rcvr withValue: zipBitBuf. interpreterProxy storeInteger: readStreamInstSize + 2 ofObject: rcvr withValue: zipBitPos. ]. interpreterProxy failed ifFalse:[ interpreterProxy pop: 5. "rcvr + args" interpreterProxy pushInteger: result. ].! ! InterpreterPlugin subclass: #InflatePlugin instanceVariableNames: 'zipCollection zipReadLimit zipPosition zipState zipBitBuf zipBitPos zipSource zipSourcePos zipSourceLimit zipLitTable zipDistTable zipCollectionSize zipLitTableSize zipDistTableSize readStreamInstSize' classVariableNames: 'MaxBits StateNoMoreData' poolDictionaries: '' category: 'VMMaker-Plugins'! !InflatePlugin commentStamp: '<historical>' prior: 0! This plugin implements the one crucial function for efficiently decompressing streams.!
!InflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 14:02'! determineSizeOfReadStream: rcvr "Determine the inst size of the class above DeflateStream by looking for the first class whose inst size is less than 14." | class | class := interpreterProxy fetchClassOf: rcvr. [class ~= interpreterProxy nilObject and: [(interpreterProxy instanceSizeOf: class) >= 14]] whileTrue: [class := interpreterProxy superclassOf: class]. class = interpreterProxy nilObject ifTrue: [^false]. readStreamInstSize := interpreterProxy instanceSizeOf: class. ^true! !
!InflatePlugin methodsFor: 'primitives' stamp: 'eem 7/18/2014 14:18'! primitiveInflateDecompressBlock "Primitive. Inflate a single block." | oop rcvr | <export: true> interpreterProxy methodArgumentCount = 2 ifFalse: [^interpreterProxy primitiveFail]. "distance table" oop := interpreterProxy stackValue: 0. (interpreterProxy isWords: oop) ifFalse: [^interpreterProxy primitiveFail]. zipDistTable := interpreterProxy firstIndexableField: oop. zipDistTableSize := interpreterProxy slotSizeOf: oop.
"literal table" oop := interpreterProxy stackValue: 1. (interpreterProxy isWords: oop) ifFalse: [^interpreterProxy primitiveFail]. zipLitTable := interpreterProxy firstIndexableField: oop. zipLitTableSize := interpreterProxy slotSizeOf: oop.
"Receiver (InflateStream)" rcvr := interpreterProxy stackValue: 2. (interpreterProxy isPointers: rcvr) ifFalse: [^interpreterProxy primitiveFail]. "All the integer instvars" readStreamInstSize = 0 ifTrue: [(self determineSizeOfReadStream: rcvr) ifFalse: [^interpreterProxy primitiveFail]. "If the receiver wasn't valid then we derived readStreamInstSize from an invalid source. discard it." (interpreterProxy slotSizeOf: rcvr) < (readStreamInstSize + 6) ifTrue: [readStreamInstSize := 0. ^interpreterProxy primitiveFail]]. (interpreterProxy slotSizeOf: rcvr) < (readStreamInstSize + 7) ifTrue: [^interpreterProxy primitiveFail].
zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. zipState := interpreterProxy fetchInteger: readStreamInstSize + 0 ofObject: rcvr. zipBitBuf := interpreterProxy fetchInteger: readStreamInstSize + 1 ofObject: rcvr. zipBitPos := interpreterProxy fetchInteger: readStreamInstSize + 2 ofObject: rcvr. zipSourcePos := interpreterProxy fetchInteger: readStreamInstSize + 4 ofObject: rcvr. zipSourceLimit := interpreterProxy fetchInteger: readStreamInstSize + 5 ofObject: rcvr. interpreterProxy failed ifTrue:[^nil]. zipReadLimit := zipReadLimit - 1. zipSourcePos := zipSourcePos - 1. zipSourceLimit := zipSourceLimit - 1.
"collection" oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^interpreterProxy primitiveFail]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
"source" oop := interpreterProxy fetchPointer: readStreamInstSize + 3 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^interpreterProxy primitiveFail]. zipSource := interpreterProxy firstIndexableField: oop.
"do the primitive" self zipDecompressBlock. interpreterProxy failed ifFalse: "store modified values back" [interpreterProxy storeInteger: 2 ofObject: rcvr withValue: zipReadLimit + 1. interpreterProxy storeInteger: readStreamInstSize + 0 ofObject: rcvr withValue: zipState. interpreterProxy storeInteger: readStreamInstSize + 1 ofObject: rcvr withValue: zipBitBuf. interpreterProxy storeInteger: readStreamInstSize + 2 ofObject: rcvr withValue: zipBitPos. interpreterProxy storeInteger: readStreamInstSize + 4 ofObject: rcvr withValue: zipSourcePos + 1. interpreterProxy pop: 2]! !
On Fri, Jul 18, 2014 at 12:46 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Fri, Jul 18, 2014 at 12:38 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
Stupid me. It's got to be the DeflateStream, and its subclasses ZipWriteStream, GZipWriteStream and ZLibWriteStream. So my change screws all uses of these in plugins. So what to do about fixing this? I would like to have a go at fixing the plugins. But cheaper might be to fix WriteStream with some property hack, storing the initialPosition somewhere else (a class side weak dictionary of stream -> initialPosition ?? (yuck)).
On Fri, Jul 18, 2014 at 12:31 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Thu, Jul 17, 2014 at 4:02 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 12:24:29AM +0200, Nicolas Cellier wrote:
2014-07-13 16:22 GMT+02:00 David T. Lewis lewis@mail.msen.com:
On Sun, Jul 13, 2014 at 09:55:41AM -0400, David T. Lewis wrote: > On Sun, Jul 06, 2014 at 12:47:25PM -0400, David T. Lewis wrote: > > On Sun, Jul 06, 2014 at 12:23:11PM -0400, David T. Lewis wrote: > > > On Sun, Jul 06, 2014 at 10:34:15AM +0200, Nicolas Cellier
wrote:
> > > > 2014-07-04 0:47 GMT+02:00 Nicolas Cellier < > > > > nicolas.cellier.aka.nice@gmail.com>: > > > > > > > > > > > > > > > > > > > > > > > > 2014-07-04 0:09 GMT+02:00 Eliot Miranda <
eliot.miranda@gmail.com
>: > > > > > > > > > > > > > > >> > > > > >> > > > > >> On Thu, Jul 3, 2014 at 12:33 PM, Nicolas Cellier < > > > > >> nicolas.cellier.aka.nice@gmail.com> wrote: > > > > >> > > > > >>> The bug is repeatable, i simply have to execute this
snippet
with my > > > > >>> test file: > > > > >>> > > > > >>> (FileStream fileNamed: 'snapshot.bin') binary contentsOfEntireFile > > > > >>> asString zipped. > > > > >>> > > > > >>> The file is too big for an attachment here - 7.3 Mbytes
- or
1.7 Mbytes > > > > >>> gzipped by external tool. > > > > >>> If someone can suggest an upload site, or want it by
mail,
just ask. > > > > >>> > > > > >> > > > > >> You're welcome to put it on ftp.mirandanamda.org,
cogftpuser,
pw cogging > > > > >> with 0's & 1's. > > > > >> > > > > >> > > > > > done, pollution of your site engaged... > > > > > Thanks Eliot! > > > > > > > > > > > > > > >> > > > > I inquired a bit more about this bug. > > > > An important clue is that it does not happen in Pharo3.0! > > > > > > > > But Pharo3.0 did not fundamentally change compression > > > > (except some FileSystem related changes, separation of CRC
stuff
in another > > > > package, some other cosmetic changes like replacing some
self
assert: ... > > > > by [...] assert and a potential bug in > > > > DeflateStream>>nextPutAll:startingAt: introduced by
CamilleTeruel).
> > > > Squeak behavior is presumably not due to a Compression
change.
> > > > > > > > Neither is it a VM problem, the bugs still shows up when
running
the Squeak > > > > image with Pharo VM. > > > > > > > > > > Yes, it is definite a problem in the Squeak trunk image. > > > > > > > > > > So the difference lies somewhere else: in our beloved
Stream.
> > > > I remembered a recent change of Eliot related to handling of Stream created > > > > with on:from:to: (Collections-eem.567). > > > > Reverting those changes just make the snippet pass! > > > > > > > > Ah ah! At the time, i found the change of Eliot quite
minimal and
nice. > > > > But I wish I had time to analyze this small innocent change
deeper.
> > > > Indeed, I remembered I previously broke my teeth on this
one 2
years or so > > > > ago, and preferred to adopt another strategy: avoid using on:from:to: and > > > > ReadWriteStream as much as possible. > > > > Why? because when analyzing the usage of inst.var. in Stream hierarchy, it > > > > gave me headaches, some subclass are just ignoring
superclass
variables, or > > > > worse are bending the semantics of superclass variables. > > > > I came to the conclusion that I could not decently master a
change
of > > > > on:from:to: > > > > > > I can not confirm this. I have an image in which I can
reliably
reproduce the > > > failure in writing the MCZ to a file. I tried reverting the
methods
that were > > > introduced in Collections-eem.567 and I am still getting the
same
failure. > > > > > > Dave > > > > Oops, I spoke too soon. Indeed the problem appears as of Collections-eem.567, > > and does not appear in Collections-nice.566. > > > > So I am confirming Nicolas' observations ... now to find the
bug :)
> > > > Dave > > > > This seems to be a rather tricky bug, and I don't think any of us
knows
the > cause right now. In case anyone else wants to have a look at it,
here is
a > recipe for reproducing the bug in a clean 4.5 image: > > - Start with a clean Squeak 4.5 image in and empty working
directory.
> ftp://ftp.squeak.org/4.5/Squeak4.5-13680.zip > > - Open the image, do not update it. > > - Open an MC browser and add the http://source.squeak.org/trunk repository for the Help-Squeak-Project package and Collections
package.
> > - Load Help-Squeak-Project-kfr.18 from trunk. > > - Load Collections-eem.567 from trunk. > > - Open a browser on class SqueakToolsDebuggerHelp, and delete the
class
side method #showDebuggerMenuForm. > > - Remove the http://source.squeak.org/trunk repository form the Help-Squeak-Project package in the MC browser. > > - Highlight your package-cache repository and try to save Help-Squeak-project to the package-cache repository. > > - Enter author initials 'dtl'. > > - For the package comment, enter 'Remove unreferenced method'. > > - Package save will fail part way through saving the MCZ. > > - To reproduce the failure in this image, do the following in a workspace: > > mcv := MCVersion allInstances detect: [:e | e name = 'a MCVersion(Help-Squeak-Project-dtl.19)']. > [f := FileStream fileNamed: 'junk.mcz'. > mcv fileOutOn: f] > ensure: [f close]. >
In addition to the above recipe, here are some other observations:
- Reverting from Collections-eem.567 to Collections-nice.566 makes
the
problem go away, but ONLY if the DeflatePlugin is active.
- The failure appears while executing the fallback code for
#primitiveDeflateBlock in ZipWriteStream>>deflateBlock:chainLength:goodMatch:
- If this primitive is deactivate by commenting out the <primitive: 'primitiveDeflateBlock' module: 'ZipPlugin'> in ZipWriteStream>>deflateBlock:chainLength:goodMatch: then the
problem will
occur regardless of which version of Collections is loaded.
- When Collections-eem.567 is loaded, the primitive will fail on
the 22nd
call to #deflateBlock:chainLength:goodMatch: and a debugger
appears
during execution of the fallback code.
- When Collections-nice.566 is loaded, the primitive does not fail
on the
22nd call to #deflateBlock:chainLength:goodMatch:, the fallback code
is never
run, and no error appears.
- When Collections-nice.566 is loaded and the primitive is disabled
to
force use of the fallback code, the error appears on on the 22nd call to #deflateBlock:chainLength:goodMatch: in exactlly the same place
as when
Collections-eem.567 is loaded.
- If I inspect the ZipWriteStream at the failure point (when the
debugger
pops up after the problem), I see no obvious difference in the state
of the
stream between the Collections-eem.567 case versus the
Collections-nice.566 with
primitive disabled case.
Dave
Very good mining work! When I hear Stream, I think fluid... But these snags make it so viscous, it's like our Stream is going to
freeze
soon. Or is it more than frozen? The vein you're after sounds as hard as
diamond,
we gonna need a sharp peak pickaxe.
I keep hoping that your stream fixes will somehow make this problem go away, but maybe we are not so lucky.
Whenever I get more time to play with this, I think I will try to catch it with gdb in the primitive. There is something strange happening that seems to first be detectable in the primitive failure, but only if Eliot's image fix is present.
This kind of problem is a nice way to spend the morning with a good cup of coffee, as some people do with the NY Times crossword puzzle ;)
http://en.wikipedia.org/wiki/The_New_York_Times_crossword_puzzle
Another "morning coffee" project is to figure out which version of the LargeIntegersPlugin should be used, which of course was the original topic that started this thread.
So given that the change adds an instance variable to WriteStream could that affect some plugin that somehow accesses a stream? What plugin could that be? I don't see any obvious inst vars in the DeflatePlugin or InflatePlugin. To remind ourselves, the three changes are
add inst var to WriteStream:
PositionableStream subclass: #WriteStream instanceVariableNames: 'writeLimit initialPositionOrNil' classVariableNames: '' poolDictionaries: '' category: 'Collections-Streams'
use it in two places: contents "Answer with a copy of my collection from the start to the current position."
readLimit := readLimit max: position. ^collection copyFrom: (initialPositionOrNil ifNil: [1]) to: position
on: aCollection from: firstIndex to: lastIndex
| len | collection := aCollection. readLimit := writeLimit := lastIndex > (len := collection size) ifTrue: [len] ifFalse: [lastIndex]. position := firstIndex <= 1 ifTrue: [0] ifFalse: [firstIndex - 1]. initialPositionOrNil := position + 1
What I've just found is that if I revert all three changes to WriteStream the bug goes away, but if I only revert the two method changes the bug remains. So I think the problem is merely the adding of the inst var. This could break some plugin which was expecting inst vars in subclasses of WriteStream to be at particular offsets determined by WriteStream instSize = 4 now being 5. The question is which plugin(s)?
Doh!
DeflatePlugin>>loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: 4 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: 5 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: 6 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: 7 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: 8 ofObject: rcvr." oop := interpreterProxy fetchPointer: 9 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 10 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 11 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 12 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: 13 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: 14 ofObject: rcvr.
^interpreterProxy failed not
I propose to add a variable that holds the inst size of the superclass of InflateStream, and to use this to correct the offsets. Objections? -- best, Eliot
Hi All,
find the fix attached. I hope we can fold this into the standard VM soon.
On Fri, Jul 18, 2014 at 2:39 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi All,
I'm close to a fix on this, but I have to stop as I've some errands torun. I wonder if anyone is interested in reviewing the code. There's a mistake in there somewhere. Careful review will help.
The issue is to match the inst var offsets in the InflatePlugin and DeflatePlugin methods with those in InflateStream DeflateStream ZipWriteStream and ZipEncoder now that the plugin code uses readStreamInstSize and writeStreamInstSize to get at them. Perhaps some kind soul can read this code like a lawyer and spot my mistake? I'll try and take another look at this this evening.
Changes attached and included for your perusal:
'From Squeak4.5 of 18 July 2014 [latest update: #13848] on 18 July 2014 at 2:34:04 pm'! InflatePlugin subclass: #DeflatePlugin instanceVariableNames: 'zipHashHead zipHashTail zipHashValue zipBlockPos zipBlockStart zipLiterals zipDistances zipLiteralFreq zipDistanceFreq zipLiteralCount zipLiteralSize zipMatchCount zipMatchLengthCodes zipDistanceCodes zipCrcTable zipExtraLengthBits zipExtraDistanceBits zipBaseLength zipBaseDistance writeStreamInstSize' classVariableNames: 'DeflateHashBits DeflateHashMask DeflateHashShift DeflateHashTableSize DeflateMaxDistance DeflateMaxDistanceCodes DeflateMaxLiteralCodes DeflateMaxMatch DeflateMinMatch DeflateWindowMask DeflateWindowSize' poolDictionaries: '' category: 'VMMaker-Plugins'! !DeflatePlugin commentStamp: 'tpr 5/5/2003 11:52' prior: 0! This adds Zip deflating support. InflatePlugin should not be translated but this subclass should since it is incorporated within that class's translation process!
!DeflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 13:56'! determineSizeOfWriteStream: rcvr "Determine the inst size of the class above DeflateStream or ZipEncoder by looking for the first class whose inst size is less than 7." | class | class := interpreterProxy fetchClassOf: rcvr. [class ~= interpreterProxy nilObject and: [(interpreterProxy instanceSizeOf: class) >= 7]] whileTrue: [class := interpreterProxy superclassOf: class]. class = interpreterProxy nilObject ifTrue: [^false]. writeStreamInstSize := interpreterProxy instanceSizeOf: class. ^true ! !
!DeflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 14:15'! loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. writeStreamInstSize = 0 ifTrue: [(self determineSizeOfWriteStream: rcvr) ifFalse: [^false]. "If the receiver wasn't valid then we derived writeStreamInstSize from an invalid source. discard it." (interpreterProxy slotSizeOf: rcvr) < (writeStreamInstSize + 5) ifTrue: [writeStreamInstSize := 0. ^false]]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: writeStreamInstSize + 0 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: writeStreamInstSize + 1 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: writeStreamInstSize + 2 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: writeStreamInstSize + 3 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: writeStreamInstSize + 4 ofObject: rcvr." oop := interpreterProxy fetchPointer: writeStreamInstSize + 5 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: writeStreamInstSize + 6 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: writeStreamInstSize + 7 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: writeStreamInstSize + 8 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: writeStreamInstSize + 9 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: writeStreamInstSize + 10 ofObject: rcvr.
^interpreterProxy failed not! !
!DeflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 14:15'! loadZipEncoderFrom: rcvr | oop | <inline: false> writeStreamInstSize = 0 ifTrue: [(self determineSizeOfWriteStream: rcvr) ifFalse: [^false]. "If the receiver wasn't valid then we derived writeStreamInstSize from an invalid source. discard it." (interpreterProxy slotSizeOf: rcvr) < (writeStreamInstSize + 3) ifTrue: [writeStreamInstSize := 0. ^false]]. ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= (writeStreamInstSize + 3)]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^interpreterProxy primitiveFail]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr." zipBitBuf := interpreterProxy fetchInteger: writeStreamInstSize + 0 ofObject: rcvr. zipBitPos := interpreterProxy fetchInteger: writeStreamInstSize + 1 ofObject: rcvr.
^interpreterProxy failed not! !
!DeflatePlugin methodsFor: 'primitives' stamp: 'eem 7/18/2014 14:28'! primitiveDeflateBlock "Primitive. Deflate the current contents of the receiver." | goodMatch chainLength lastIndex rcvr result | <export: true> <inline: false> interpreterProxy methodArgumentCount = 3 ifFalse:[^interpreterProxy primitiveFail]. goodMatch := interpreterProxy stackIntegerValue: 0. chainLength := interpreterProxy stackIntegerValue: 1. lastIndex := interpreterProxy stackIntegerValue: 2. rcvr := interpreterProxy stackObjectValue: 3. interpreterProxy failed ifTrue:[^nil]. self cCode:'' inSmalltalk:[ zipMatchLengthCodes := CArrayAccessor on: ZipWriteStream matchLengthCodes. zipDistanceCodes := CArrayAccessor on: ZipWriteStream distanceCodes]. (self loadDeflateStreamFrom: rcvr) ifFalse:[^interpreterProxy primitiveFail]. result := self deflateBlock: lastIndex chainLength: chainLength goodMatch: goodMatch. interpreterProxy failed ifFalse:[ "Store back modified values" interpreterProxy storeInteger: writeStreamInstSize + 2 ofObject: rcvr withValue: zipHashValue. interpreterProxy storeInteger: writeStreamInstSize + 3 ofObject: rcvr withValue: zipBlockPos. interpreterProxy storeInteger: writeStreamInstSize + 9 ofObject: rcvr withValue: zipLiteralCount. interpreterProxy storeInteger: writeStreamInstSize + 10 ofObject: rcvr withValue: zipMatchCount]. interpreterProxy failed ifFalse:[ interpreterProxy pop: 4. interpreterProxy pushBool: result. ].! !
!DeflatePlugin methodsFor: 'primitives' stamp: 'eem 7/18/2014 14:24'! primitiveZipSendBlock | distTree litTree distStream litStream rcvr result | <export: true> interpreterProxy methodArgumentCount = 4 ifFalse:[^interpreterProxy primitiveFail]. distTree := interpreterProxy stackObjectValue: 0. litTree := interpreterProxy stackObjectValue: 1. distStream := interpreterProxy stackObjectValue: 2. litStream := interpreterProxy stackObjectValue: 3. rcvr := interpreterProxy stackObjectValue: 4. interpreterProxy failed ifTrue:[^nil]. (self loadZipEncoderFrom: rcvr) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: distTree) and:[ (interpreterProxy slotSizeOf: distTree) >= 2]) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: litTree) and:[ (interpreterProxy slotSizeOf: litTree) >= 2]) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: litStream) and:[ (interpreterProxy slotSizeOf: litStream) >= 3]) ifFalse:[^interpreterProxy primitiveFail]. ((interpreterProxy isPointers: distStream) and:[ (interpreterProxy slotSizeOf: distStream) >= 3]) ifFalse:[^interpreterProxy primitiveFail]. self cCode:'' inSmalltalk:[ zipMatchLengthCodes := CArrayAccessor on: ZipWriteStream matchLengthCodes. zipDistanceCodes := CArrayAccessor on: ZipWriteStream distanceCodes. zipExtraLengthBits := CArrayAccessor on: ZipWriteStream extraLengthBits. zipExtraDistanceBits := CArrayAccessor on: ZipWriteStream extraDistanceBits. zipBaseLength := CArrayAccessor on: ZipWriteStream baseLength. zipBaseDistance := CArrayAccessor on: ZipWriteStream baseDistance]. result := self sendBlock: litStream with: distStream with: litTree with: distTree. interpreterProxy failed ifFalse:[ interpreterProxy storeInteger: 1 ofObject: rcvr withValue: zipPosition. interpreterProxy storeInteger: readStreamInstSize + 1 ofObject: rcvr withValue: zipBitBuf. interpreterProxy storeInteger: readStreamInstSize + 2 ofObject: rcvr withValue: zipBitPos. ]. interpreterProxy failed ifFalse:[ interpreterProxy pop: 5. "rcvr + args" interpreterProxy pushInteger: result. ].! ! InterpreterPlugin subclass: #InflatePlugin instanceVariableNames: 'zipCollection zipReadLimit zipPosition zipState zipBitBuf zipBitPos zipSource zipSourcePos zipSourceLimit zipLitTable zipDistTable zipCollectionSize zipLitTableSize zipDistTableSize readStreamInstSize' classVariableNames: 'MaxBits StateNoMoreData' poolDictionaries: '' category: 'VMMaker-Plugins'! !InflatePlugin commentStamp: '<historical>' prior: 0! This plugin implements the one crucial function for efficiently decompressing streams.!
!InflatePlugin methodsFor: 'primitive support' stamp: 'eem 7/18/2014 14:02'! determineSizeOfReadStream: rcvr "Determine the inst size of the class above DeflateStream by looking for the first class whose inst size is less than 14." | class | class := interpreterProxy fetchClassOf: rcvr. [class ~= interpreterProxy nilObject and: [(interpreterProxy instanceSizeOf: class) >= 14]] whileTrue: [class := interpreterProxy superclassOf: class]. class = interpreterProxy nilObject ifTrue: [^false]. readStreamInstSize := interpreterProxy instanceSizeOf: class. ^true! !
!InflatePlugin methodsFor: 'primitives' stamp: 'eem 7/18/2014 14:18'! primitiveInflateDecompressBlock "Primitive. Inflate a single block." | oop rcvr | <export: true> interpreterProxy methodArgumentCount = 2 ifFalse: [^interpreterProxy primitiveFail]. "distance table" oop := interpreterProxy stackValue: 0. (interpreterProxy isWords: oop) ifFalse: [^interpreterProxy primitiveFail]. zipDistTable := interpreterProxy firstIndexableField: oop. zipDistTableSize := interpreterProxy slotSizeOf: oop.
"literal table" oop := interpreterProxy stackValue: 1. (interpreterProxy isWords: oop) ifFalse: [^interpreterProxy primitiveFail]. zipLitTable := interpreterProxy firstIndexableField: oop. zipLitTableSize := interpreterProxy slotSizeOf: oop.
"Receiver (InflateStream)" rcvr := interpreterProxy stackValue: 2. (interpreterProxy isPointers: rcvr) ifFalse: [^interpreterProxy primitiveFail]. "All the integer instvars" readStreamInstSize = 0 ifTrue: [(self determineSizeOfReadStream: rcvr) ifFalse: [^interpreterProxy primitiveFail]. "If the receiver wasn't valid then we derived readStreamInstSize from an invalid source. discard it." (interpreterProxy slotSizeOf: rcvr) < (readStreamInstSize + 6) ifTrue: [readStreamInstSize := 0. ^interpreterProxy primitiveFail]]. (interpreterProxy slotSizeOf: rcvr) < (readStreamInstSize + 7) ifTrue: [^interpreterProxy primitiveFail].
zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. zipState := interpreterProxy fetchInteger: readStreamInstSize + 0 ofObject: rcvr. zipBitBuf := interpreterProxy fetchInteger: readStreamInstSize + 1 ofObject: rcvr. zipBitPos := interpreterProxy fetchInteger: readStreamInstSize + 2 ofObject: rcvr. zipSourcePos := interpreterProxy fetchInteger: readStreamInstSize + 4 ofObject: rcvr. zipSourceLimit := interpreterProxy fetchInteger: readStreamInstSize + 5 ofObject: rcvr. interpreterProxy failed ifTrue:[^nil]. zipReadLimit := zipReadLimit - 1. zipSourcePos := zipSourcePos - 1. zipSourceLimit := zipSourceLimit - 1.
"collection" oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^interpreterProxy primitiveFail]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
"source" oop := interpreterProxy fetchPointer: readStreamInstSize + 3 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^interpreterProxy primitiveFail]. zipSource := interpreterProxy firstIndexableField: oop.
"do the primitive" self zipDecompressBlock. interpreterProxy failed ifFalse: "store modified values back" [interpreterProxy storeInteger: 2 ofObject: rcvr withValue: zipReadLimit
interpreterProxy storeInteger: readStreamInstSize + 0 ofObject: rcvr withValue: zipState. interpreterProxy storeInteger: readStreamInstSize + 1 ofObject: rcvr withValue: zipBitBuf. interpreterProxy storeInteger: readStreamInstSize + 2 ofObject: rcvr withValue: zipBitPos. interpreterProxy storeInteger: readStreamInstSize + 4 ofObject: rcvr withValue: zipSourcePos + 1. interpreterProxy pop: 2]! !
On Fri, Jul 18, 2014 at 12:46 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Fri, Jul 18, 2014 at 12:38 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
Stupid me. It's got to be the DeflateStream, and its subclasses ZipWriteStream, GZipWriteStream and ZLibWriteStream. So my change screws all uses of these in plugins. So what to do about fixing this? I would like to have a go at fixing the plugins. But cheaper might be to fix WriteStream with some property hack, storing the initialPosition somewhere else (a class side weak dictionary of stream -> initialPosition ?? (yuck)).
On Fri, Jul 18, 2014 at 12:31 PM, Eliot Miranda <eliot.miranda@gmail.com
wrote:
On Thu, Jul 17, 2014 at 4:02 PM, David T. Lewis lewis@mail.msen.com wrote:
On Fri, Jul 18, 2014 at 12:24:29AM +0200, Nicolas Cellier wrote:
2014-07-13 16:22 GMT+02:00 David T. Lewis lewis@mail.msen.com:
> On Sun, Jul 13, 2014 at 09:55:41AM -0400, David T. Lewis wrote: > > On Sun, Jul 06, 2014 at 12:47:25PM -0400, David T. Lewis wrote: > > > On Sun, Jul 06, 2014 at 12:23:11PM -0400, David T. Lewis wrote: > > > > On Sun, Jul 06, 2014 at 10:34:15AM +0200, Nicolas Cellier
wrote:
> > > > > 2014-07-04 0:47 GMT+02:00 Nicolas Cellier < > > > > > nicolas.cellier.aka.nice@gmail.com>: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2014-07-04 0:09 GMT+02:00 Eliot Miranda <
eliot.miranda@gmail.com
> >: > > > > > > > > > > > > > > > > > >> > > > > > >> > > > > > >> On Thu, Jul 3, 2014 at 12:33 PM, Nicolas Cellier < > > > > > >> nicolas.cellier.aka.nice@gmail.com> wrote: > > > > > >> > > > > > >>> The bug is repeatable, i simply have to execute this
snippet
> with my > > > > > >>> test file: > > > > > >>> > > > > > >>> (FileStream fileNamed: 'snapshot.bin') binary > contentsOfEntireFile > > > > > >>> asString zipped. > > > > > >>> > > > > > >>> The file is too big for an attachment here - 7.3
Mbytes - or
> 1.7 Mbytes > > > > > >>> gzipped by external tool. > > > > > >>> If someone can suggest an upload site, or want it by
mail,
> just ask. > > > > > >>> > > > > > >> > > > > > >> You're welcome to put it on ftp.mirandanamda.org,
cogftpuser,
> pw cogging > > > > > >> with 0's & 1's. > > > > > >> > > > > > >> > > > > > > done, pollution of your site engaged... > > > > > > Thanks Eliot! > > > > > > > > > > > > > > > > > >> > > > > > I inquired a bit more about this bug. > > > > > An important clue is that it does not happen in Pharo3.0! > > > > > > > > > > But Pharo3.0 did not fundamentally change compression > > > > > (except some FileSystem related changes, separation of CRC
stuff
> in another > > > > > package, some other cosmetic changes like replacing some
self
> assert: ... > > > > > by [...] assert and a potential bug in > > > > > DeflateStream>>nextPutAll:startingAt: introduced by
CamilleTeruel).
> > > > > Squeak behavior is presumably not due to a Compression
change.
> > > > > > > > > > Neither is it a VM problem, the bugs still shows up when
running
> the Squeak > > > > > image with Pharo VM. > > > > > > > > > > > > > Yes, it is definite a problem in the Squeak trunk image. > > > > > > > > > > > > > So the difference lies somewhere else: in our beloved
Stream.
> > > > > I remembered a recent change of Eliot related to handling
of
> Stream created > > > > > with on:from:to: (Collections-eem.567). > > > > > Reverting those changes just make the snippet pass! > > > > > > > > > > Ah ah! At the time, i found the change of Eliot quite
minimal and
> nice. > > > > > But I wish I had time to analyze this small innocent
change deeper.
> > > > > Indeed, I remembered I previously broke my teeth on this
one 2
> years or so > > > > > ago, and preferred to adopt another strategy: avoid using > on:from:to: and > > > > > ReadWriteStream as much as possible. > > > > > Why? because when analyzing the usage of inst.var. in
Stream
> hierarchy, it > > > > > gave me headaches, some subclass are just ignoring
superclass
> variables, or > > > > > worse are bending the semantics of superclass variables. > > > > > I came to the conclusion that I could not decently master
a change
> of > > > > > on:from:to: > > > > > > > > I can not confirm this. I have an image in which I can
reliably
> reproduce the > > > > failure in writing the MCZ to a file. I tried reverting the
methods
> that were > > > > introduced in Collections-eem.567 and I am still getting the
same
> failure. > > > > > > > > Dave > > > > > > Oops, I spoke too soon. Indeed the problem appears as of > Collections-eem.567, > > > and does not appear in Collections-nice.566. > > > > > > So I am confirming Nicolas' observations ... now to find the
bug :)
> > > > > > Dave > > > > > > > This seems to be a rather tricky bug, and I don't think any of
us knows
> the > > cause right now. In case anyone else wants to have a look at it,
here is
> a > > recipe for reproducing the bug in a clean 4.5 image: > > > > - Start with a clean Squeak 4.5 image in and empty working
directory.
> > ftp://ftp.squeak.org/4.5/Squeak4.5-13680.zip > > > > - Open the image, do not update it. > > > > - Open an MC browser and add the http://source.squeak.org/trunk > repository for the Help-Squeak-Project package and Collections
package.
> > > > - Load Help-Squeak-Project-kfr.18 from trunk. > > > > - Load Collections-eem.567 from trunk. > > > > - Open a browser on class SqueakToolsDebuggerHelp, and delete
the class
> side method #showDebuggerMenuForm. > > > > - Remove the http://source.squeak.org/trunk repository form the > Help-Squeak-Project package in the MC browser. > > > > - Highlight your package-cache repository and try to save > Help-Squeak-project to the package-cache repository. > > > > - Enter author initials 'dtl'. > > > > - For the package comment, enter 'Remove unreferenced method'. > > > > - Package save will fail part way through saving the MCZ. > > > > - To reproduce the failure in this image, do the following in a > workspace: > > > > mcv := MCVersion allInstances detect: [:e | e name = 'a > MCVersion(Help-Squeak-Project-dtl.19)']. > > [f := FileStream fileNamed: 'junk.mcz'. > > mcv fileOutOn: f] > > ensure: [f close]. > > > > In addition to the above recipe, here are some other observations: > > - Reverting from Collections-eem.567 to Collections-nice.566 makes
the
> problem go away, but ONLY if the DeflatePlugin is active. > > - The failure appears while executing the fallback code for > #primitiveDeflateBlock > in ZipWriteStream>>deflateBlock:chainLength:goodMatch: > > - If this primitive is deactivate by commenting out the > <primitive: 'primitiveDeflateBlock' module: 'ZipPlugin'> in > ZipWriteStream>>deflateBlock:chainLength:goodMatch: then the
problem will
> occur regardless of which version of Collections is loaded. > > - When Collections-eem.567 is loaded, the primitive will fail on
the 22nd
> call to #deflateBlock:chainLength:goodMatch: and a debugger
appears
> during > execution of the fallback code. > > - When Collections-nice.566 is loaded, the primitive does not fail
on the
> 22nd > call to #deflateBlock:chainLength:goodMatch:, the fallback code
is never
> run, > and no error appears. > > - When Collections-nice.566 is loaded and the primitive is
disabled to
> force > use of the fallback code, the error appears on on the 22nd call
to
> #deflateBlock:chainLength:goodMatch: in exactlly the same place
as when
> Collections-eem.567 is loaded. > > - If I inspect the ZipWriteStream at the failure point (when the
debugger
> pops > up after the problem), I see no obvious difference in the state
of the
> stream > between the Collections-eem.567 case versus the
Collections-nice.566 with
> primitive disabled case. > > Dave > > Very good mining work! When I hear Stream, I think fluid... But these snags make it so viscous, it's like our Stream is going to
freeze
soon. Or is it more than frozen? The vein you're after sounds as hard as
diamond,
we gonna need a sharp peak pickaxe.
I keep hoping that your stream fixes will somehow make this problem go away, but maybe we are not so lucky.
Whenever I get more time to play with this, I think I will try to catch it with gdb in the primitive. There is something strange happening that seems to first be detectable in the primitive failure, but only if Eliot's image fix is present.
This kind of problem is a nice way to spend the morning with a good cup of coffee, as some people do with the NY Times crossword puzzle ;)
http://en.wikipedia.org/wiki/The_New_York_Times_crossword_puzzle
Another "morning coffee" project is to figure out which version of the LargeIntegersPlugin should be used, which of course was the original topic that started this thread.
So given that the change adds an instance variable to WriteStream could that affect some plugin that somehow accesses a stream? What plugin could that be? I don't see any obvious inst vars in the DeflatePlugin or InflatePlugin. To remind ourselves, the three changes are
add inst var to WriteStream:
PositionableStream subclass: #WriteStream instanceVariableNames: 'writeLimit initialPositionOrNil' classVariableNames: '' poolDictionaries: '' category: 'Collections-Streams'
use it in two places: contents "Answer with a copy of my collection from the start to the current position."
readLimit := readLimit max: position. ^collection copyFrom: (initialPositionOrNil ifNil: [1]) to: position
on: aCollection from: firstIndex to: lastIndex
| len | collection := aCollection. readLimit := writeLimit := lastIndex > (len := collection size) ifTrue: [len] ifFalse: [lastIndex]. position := firstIndex <= 1 ifTrue: [0] ifFalse: [firstIndex - 1]. initialPositionOrNil := position + 1
What I've just found is that if I revert all three changes to WriteStream the bug goes away, but if I only revert the two method changes the bug remains. So I think the problem is merely the adding of the inst var. This could break some plugin which was expecting inst vars in subclasses of WriteStream to be at particular offsets determined by WriteStream instSize = 4 now being 5. The question is which plugin(s)?
Doh!
DeflatePlugin>>loadDeflateStreamFrom: rcvr | oop | <inline: false> ((interpreterProxy isPointers: rcvr) and: [(interpreterProxy slotSizeOf: rcvr) >= 15]) ifFalse: [^false]. oop := interpreterProxy fetchPointer: 0 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. "zipWriteLimit := interpreterProxy fetchInteger: 3 ofObject: rcvr."
oop := interpreterProxy fetchPointer: 4 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: 5 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: 6 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: 7 ofObject: rcvr. "zipBlockStart := interpreterProxy fetchInteger: 8 ofObject: rcvr." oop := interpreterProxy fetchPointer: 9 ofObject: rcvr. (interpreterProxy isBytes: oop) ifFalse: [^false]. zipLiteralSize := interpreterProxy slotSizeOf: oop. zipLiterals := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 10 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) >= zipLiteralSize]) ifFalse: [^false]. zipDistances := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 11 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxLiteralCodes]) ifFalse: [^false]. zipLiteralFreq := interpreterProxy firstIndexableField: oop.
oop := interpreterProxy fetchPointer: 12 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateMaxDistanceCodes]) ifFalse: [^false]. zipDistanceFreq := interpreterProxy firstIndexableField: oop.
zipLiteralCount := interpreterProxy fetchInteger: 13 ofObject: rcvr. zipMatchCount := interpreterProxy fetchInteger: 14 ofObject: rcvr.
^interpreterProxy failed not
I propose to add a variable that holds the inst size of the superclass of InflateStream, and to use this to correct the offsets. Objections? -- best, Eliot
-- best, Eliot
On Fri, Jul 18, 2014 at 05:26:52PM -0700, Eliot Miranda wrote:
Hi All,
find the fix attached. I hope we can fold this into the standard VM soon.
Thanks Eliot, I'll update it this weekend in VMM trunk also.
Dave
This is a good workaround for the problem, but I think the main issue is that the VM has assumptions about the layout of an object. I would simply pass all the parameters to the primitive. There's only 14 of them, so this could work. But this plugin also writes back the state to the object, so that would create another problem.
I think the best would be to create new variants of #primitiveZipSendBlock and #primitiveDeflateBlock which accept an array, and use that to load and store the paramters of the compression algorithm. This way objects with different layout would be able to use the plugin, and we could get rid of these compression streams in the long term (or change their layout without breaking them).
Another solution would be to use an external library(libraries) for compression, but that's a lot more work.
Levente
P.S.: instead of finding the number of slots of WriteStream, it would have been possible to index the slots from the end of the passed object. E.g.
loadDeflateStreamFrom: rcvr | slotSize oop | <inline: false> (interpreterProxy isPointers: rcvr) ifFalse: [ ^false ]. slotSize := interpreterProxy slotSizeOf: rcvr. slotSize < 15 ifTrue: [ ^false ]. ... zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop.
zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. ... oop := interpreterProxy fetchPointer: slotSize - 14 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize]) ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: slotSize - 13 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize]) ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: slotSize - 12 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: slotSize - 11 ofObject: rcvr. ...
Of course this would have its own cons.
On Sat, 19 Jul 2014, David T. Lewis wrote:
On Fri, Jul 18, 2014 at 05:26:52PM -0700, Eliot Miranda wrote:
Hi All,
find the fix attached. I hope we can fold this into the standard VM soon.
Thanks Eliot, I'll update it this weekend in VMM trunk also.
Dave
On Sat, Jul 19, 2014 at 11:42 AM, Levente Uzonyi leves@elte.hu wrote:
This is a good workaround for the problem, but I think the main issue is that the VM has assumptions about the layout of an object. I would simply pass all the parameters to the primitive. There's only 14 of them, so this could work. But this plugin also writes back the state to the object, so that would create another problem.
Yes, better is an indexOfInstVarNamed: interface where the indices are computed once on first use. But this doesn't exist as yet.
I think the best would be to create new variants of #primitiveZipSendBlock
and #primitiveDeflateBlock which accept an array, and use that to load and store the paramters of the compression algorithm. This way objects with different layout would be able to use the plugin, and we could get rid of these compression streams in the long term (or change their layout without breaking them).
Best is Sista speculative inlining/adaptive optimization, where the vanilla Smalltalk code is optimized to equal or exceed C level performance. We might be as little as two years away from that.
Another solution would be to use an external library(libraries) for compression, but that's a lot more work.
Levente
P.S.: instead of finding the number of slots of WriteStream, it would have been possible to index the slots from the end of the passed object. E.g.
loadDeflateStreamFrom: rcvr | slotSize oop | <inline: false> (interpreterProxy isPointers: rcvr) ifFalse: [ ^false ]. slotSize := interpreterProxy slotSizeOf: rcvr. slotSize < 15 ifTrue: [ ^false ]. ...
zipCollection := interpreterProxy firstIndexableField: oop. zipCollectionSize := interpreterProxy byteSizeOf: oop. zipPosition := interpreterProxy fetchInteger: 1 ofObject: rcvr. zipReadLimit := interpreterProxy fetchInteger: 2 ofObject: rcvr. ... oop := interpreterProxy fetchPointer: slotSize - 14 ofObject: rcvr. ((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateHashTableSize])ifFalse: [^false]. zipHashHead := interpreterProxy firstIndexableField: oop. oop := interpreterProxy fetchPointer: slotSize - 13 ofObject: rcvr.
((interpreterProxy isWords: oop) and: [(interpreterProxy slotSizeOf: oop) = DeflateWindowSize])ifFalse: [^false]. zipHashTail := interpreterProxy firstIndexableField: oop. zipHashValue := interpreterProxy fetchInteger: slotSize - 12 ofObject: rcvr. zipBlockPos := interpreterProxy fetchInteger: slotSize - 11 ofObject: rcvr. ...
Of course this would have its own cons.
On Sat, 19 Jul 2014, David T. Lewis wrote:
On Fri, Jul 18, 2014 at 05:26:52PM -0700, Eliot Miranda wrote:
Hi All,
find the fix attached. I hope we can fold this into the standard VM soon.
Thanks Eliot, I'll update it this weekend in VMM trunk also.
Dave
vm-dev@lists.squeakfoundation.org
-
Bert Freudenberg -
David T. Lewis -
Eliot Miranda -
Levente Uzonyi -
Louis LaBrunda -
tim Rowledge