Squeak 4.6 release update

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Squeak 4.6 release update

Chris Muller-4
Besides welcoming Marcel to the board yesterday, we discussed that
we'd like to push for the 4.6 / 5.0 release by April 30th.  Somehow my
name was the only one to be found on the volunteer list as release
manager, but Eliot voiced his willingness to assist, but will be
relying on others to chip in as well.

So, I've just updated the Squeak 4.5-13686 image from trunk (lo and
behold, it worked without a hitch!) to produce a Squeak4.6-14191.zip
which is now available at ftp.squeak.org/4.6alpha/.

>From this snapshot forward, lets start thinking about the 4.6 release
and all that we want to be in it.  If you have any low-level changes
to the image that would benefit from as much testing as possible, now
is the time to get those finalized and committed into the trunk.
Controversial changes should start in the Inbox as always, we still
have time for one or two more arguments.  :)

In a few weeks, we'll want to start being more conservative with our
changes; only things like tools and cosmetics -- and we'll hammer out
the look and UX.

4.6 is a dual release also as 5.0.  We'll be releasing two VM's and
two images; one each for the Cog and Spur formats.  The content of the
images will be equivalent.

tty
Reply | Threaded
Open this post in threaded view
|

Re: Squeak 4.6 release update

tty
The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:

---- On Fri, 06 Mar 2015 15:23:00 -0500 Chris Muller<[hidden email]> wrote ----
Besides welcoming Marcel to the board yesterday, we discussed that
we'd like to push for the 4.6 / 5.0 release by April 30th. Somehow my
name was the only one to be found on the volunteer list as release
manager, but Eliot voiced his willingness to assist, but will be
relying on others to chip in as well.

So, I've just updated the Squeak 4.5-13686 image from trunk (lo and
behold, it worked without a hitch!) to produce a Squeak4.6-14191.zip
which is now available at ftp.squeak.org/4.6alpha/.

>From this snapshot forward, lets start thinking about the 4.6 release
and all that we want to be in it. If you have any low-level changes
to the image that would benefit from as much testing as possible, now
is the time to get those finalized and committed into the trunk.
Controversial changes should start in the Inbox as always, we still
have time for one or two more arguments. :)

In a few weeks, we'll want to start being more conservative with our
changes; only things like tools and cosmetics -- and we'll hammer out
the look and UX.

4.6 is a dual release also as 5.0. We'll be releasing two VM's and
two images; one each for the Cog and Spur formats. The content of the
images will be equivalent.





Reply | Threaded
Open this post in threaded view
|

Re: Squeak 4.6 release update

Tobias Pape
Hey,
On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:

> The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:

to which fix do you refer?
There is a hack out there calling flush every now and then,
but this all calls for a principled solution for streams that
read _and_ write.

best
        -Tobias


>
> ---- On Fri, 06 Mar 2015 15:23:00 -0500 Chris Muller<[hidden email]> wrote ----
> Besides welcoming Marcel to the board yesterday, we discussed that
> we'd like to push for the 4.6 / 5.0 release by April 30th. Somehow my
> name was the only one to be found on the volunteer list as release
> manager, but Eliot voiced his willingness to assist, but will be
> relying on others to chip in as well.
>
> So, I've just updated the Squeak 4.5-13686 image from trunk (lo and
> behold, it worked without a hitch!) to produce a Squeak4.6-14191.zip
> which is now available at ftp.squeak.org/4.6alpha/.
>
>> From this snapshot forward, lets start thinking about the 4.6 release
> and all that we want to be in it. If you have any low-level changes
> to the image that would benefit from as much testing as possible, now
> is the time to get those finalized and committed into the trunk.
> Controversial changes should start in the Inbox as always, we still
> have time for one or two more arguments. :)
>
> In a few weeks, we'll want to start being more conservative with our
> changes; only things like tools and cosmetics -- and we'll hammer out
> the look and UX.
>
> 4.6 is a dual release also as 5.0. We'll be releasing two VM's and
> two images; one each for the Cog and Spur formats. The content of the
> images will be equivalent.



Reply | Threaded
Open this post in threaded view
|

Re: Squeak 4.6 release update

David T. Lewis
On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
> Hey,
> On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
>
> > The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:
>
> to which fix do you refer?
> There is a hack out there calling flush every now and then,
> but this all calls for a principled solution for streams that
> read _and_ write.

You are right that it is a hack, but the actual problem is a defect
in some C runtime libraries, notably on Ubuntu but possibly others
as well.

What we really need is a unit test that captures the bug. I was not able
to come up with one myself, but it would be quite helpful if someone
could do it.

In addition to the error saving comments, I also have a large number
of failures in my OSProcess and CommandShell tests when running on
my Ubuntu laptop with the buggy C runtime. I cannot prove it, but I
am fairly sure it is the same underlying Ubuntu bug causing these
problems.

Dave


>
> best
> -Tobias
>
>
> >
> > ---- On Fri, 06 Mar 2015 15:23:00 -0500 Chris Muller<[hidden email]> wrote ----
> > Besides welcoming Marcel to the board yesterday, we discussed that
> > we'd like to push for the 4.6 / 5.0 release by April 30th. Somehow my
> > name was the only one to be found on the volunteer list as release
> > manager, but Eliot voiced his willingness to assist, but will be
> > relying on others to chip in as well.
> >
> > So, I've just updated the Squeak 4.5-13686 image from trunk (lo and
> > behold, it worked without a hitch!) to produce a Squeak4.6-14191.zip
> > which is now available at ftp.squeak.org/4.6alpha/.
> >
> >> From this snapshot forward, lets start thinking about the 4.6 release
> > and all that we want to be in it. If you have any low-level changes
> > to the image that would benefit from as much testing as possible, now
> > is the time to get those finalized and committed into the trunk.
> > Controversial changes should start in the Inbox as always, we still
> > have time for one or two more arguments. :)
> >
> > In a few weeks, we'll want to start being more conservative with our
> > changes; only things like tools and cosmetics -- and we'll hammer out
> > the look and UX.
> >
> > 4.6 is a dual release also as 5.0. We'll be releasing two VM's and
> > two images; one each for the Cog and Spur formats. The content of the
> > images will be equivalent.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Squeak 4.6 release update

Levente Uzonyi-2
On Sat, 7 Mar 2015, David T. Lewis wrote:

> On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
>> Hey,
>> On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
>>
>>> The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:
>>
>> to which fix do you refer?
>> There is a hack out there calling flush every now and then,
>> but this all calls for a principled solution for streams that
>> read _and_ write.
>
> You are right that it is a hack, but the actual problem is a defect
> in some C runtime libraries, notably on Ubuntu but possibly others
> as well.

Are you sure it's a bug in the OS? Isn't it just the allocate-on-flush
behavior? https://en.wikipedia.org/wiki/Allocate-on-flush

Levente

>
> What we really need is a unit test that captures the bug. I was not able
> to come up with one myself, but it would be quite helpful if someone
> could do it.
>
> In addition to the error saving comments, I also have a large number
> of failures in my OSProcess and CommandShell tests when running on
> my Ubuntu laptop with the buggy C runtime. I cannot prove it, but I
> am fairly sure it is the same underlying Ubuntu bug causing these
> problems.
>
> Dave
>
>
>>
>> best
>> -Tobias
>>
>>
>>>
>>> ---- On Fri, 06 Mar 2015 15:23:00 -0500 Chris Muller<[hidden email]> wrote ----
>>> Besides welcoming Marcel to the board yesterday, we discussed that
>>> we'd like to push for the 4.6 / 5.0 release by April 30th. Somehow my
>>> name was the only one to be found on the volunteer list as release
>>> manager, but Eliot voiced his willingness to assist, but will be
>>> relying on others to chip in as well.
>>>
>>> So, I've just updated the Squeak 4.5-13686 image from trunk (lo and
>>> behold, it worked without a hitch!) to produce a Squeak4.6-14191.zip
>>> which is now available at ftp.squeak.org/4.6alpha/.
>>>
>>>> From this snapshot forward, lets start thinking about the 4.6 release
>>> and all that we want to be in it. If you have any low-level changes
>>> to the image that would benefit from as much testing as possible, now
>>> is the time to get those finalized and committed into the trunk.
>>> Controversial changes should start in the Inbox as always, we still
>>> have time for one or two more arguments. :)
>>>
>>> In a few weeks, we'll want to start being more conservative with our
>>> changes; only things like tools and cosmetics -- and we'll hammer out
>>> the look and UX.
>>>
>>> 4.6 is a dual release also as 5.0. We'll be releasing two VM's and
>>> two images; one each for the Cog and Spur formats. The content of the
>>> images will be equivalent.
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Squeak 4.6 release update

Tobias Pape
Hi,

On 08.03.2015, at 04:18, Levente Uzonyi <[hidden email]> wrote:

> On Sat, 7 Mar 2015, David T. Lewis wrote:
>
>> On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
>>> Hey,
>>> On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
>>>
>>>> The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:
>>>
>>> to which fix do you refer?
>>> There is a hack out there calling flush every now and then,
>>> but this all calls for a principled solution for streams that
>>> read _and_ write.
>>
>> You are right that it is a hack, but the actual problem is a defect
>> in some C runtime libraries, notably on Ubuntu but possibly others
>> as well.
>
> Are you sure it's a bug in the OS? Isn't it just the allocate-on-flush behavior?https://en.wikipedia.org/wiki/Allocate-on-flush

I too doubt an OS bug.
Someone (i don't remember) said to me, that the behavior of the c calls
make clear not to rely on certain thing.
I plan to investigate the issue soonish.

best
        -tobias
Reply | Threaded
Open this post in threaded view
|

Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

David T. Lewis
On Sun, Mar 08, 2015 at 11:44:18AM +0100, Tobias Pape wrote:

> Hi,
>
> On 08.03.2015, at 04:18, Levente Uzonyi <[hidden email]> wrote:
>
> > On Sat, 7 Mar 2015, David T. Lewis wrote:
> >
> >> On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
> >>> Hey,
> >>> On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
> >>>
> >>>> The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:
> >>>
> >>> to which fix do you refer?
> >>> There is a hack out there calling flush every now and then,
> >>> but this all calls for a principled solution for streams that
> >>> read _and_ write.
> >>
> >> You are right that it is a hack, but the actual problem is a defect
> >> in some C runtime libraries, notably on Ubuntu but possibly others
> >> as well.
> >
> > Are you sure it's a bug in the OS? Isn't it just the allocate-on-flush behavior?https://en.wikipedia.org/wiki/Allocate-on-flush

No, I am not sure. I had convinced myself that it was a libc issue, but I may be wrong.

>
> I too doubt an OS bug.
> Someone (i don't remember) said to me, that the behavior of the c calls
> make clear not to rely on certain thing.
> I plan to investigate the issue soonish.
>
> best
> -tobias

That would be great.

The workaround that "fixes" the comment problem is in WriteStream>>nextChunkPut:,
so if you remove the #flush at the end of this method, the problem can be
reproduced. At the time, I was not able to come up with a unit test that would
reproduce the problem, but my best guess as to what was happening is in the
update comment:


  Name: Collections-dtl.568
  Author: dtl
  Time: 5 May 2014, 12:39:30.026 pm
 
  Add a flush to WriteStream>>nextChunkPut:
 
  This is a workaround for a bug in the runtime library of some versions of
  Ubuntu. The symptom is that creation of a class comment for a class that
  previously had no comment leads to a file size error in the new RemoteStream
  that points to the class comment. Actual file size and contents of the
  changes file are not affected by this bug, and the error occurs when reading
  contents of the changes file immediately following the initial save, Flushing
  the stream after writing a chunk to the changes file prevents the problem.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

David T. Lewis
On Sun, Mar 08, 2015 at 11:29:18AM -0400, David T. Lewis wrote:

> On Sun, Mar 08, 2015 at 11:44:18AM +0100, Tobias Pape wrote:
> > Hi,
> >
> > On 08.03.2015, at 04:18, Levente Uzonyi <[hidden email]> wrote:
> >
> > > On Sat, 7 Mar 2015, David T. Lewis wrote:
> > >
> > >> On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
> > >>> Hey,
> > >>> On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
> > >>>
> > >>>> The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:
> > >>>
> > >>> to which fix do you refer?
> > >>> There is a hack out there calling flush every now and then,
> > >>> but this all calls for a principled solution for streams that
> > >>> read _and_ write.
> > >>
> > >> You are right that it is a hack, but the actual problem is a defect
> > >> in some C runtime libraries, notably on Ubuntu but possibly others
> > >> as well.
> > >
> > > Are you sure it's a bug in the OS? Isn't it just the allocate-on-flush behavior?https://en.wikipedia.org/wiki/Allocate-on-flush
>
> No, I am not sure. I had convinced myself that it was a libc issue, but I may be wrong.
>

To check this, I tried running an image from a thumb drive with vfat file
system, which presumably does not have the allocate-on-flush feature. The
bug is still present (confirmed by reverting WriteStream>>nextChunkPut: to
the earlier version). So the problem does not appear to be associated with
the file system.

Dave



> >
> > I too doubt an OS bug.
> > Someone (i don't remember) said to me, that the behavior of the c calls
> > make clear not to rely on certain thing.
> > I plan to investigate the issue soonish.
> >
> > best
> > -tobias
>
> That would be great.
>
> The workaround that "fixes" the comment problem is in WriteStream>>nextChunkPut:,
> so if you remove the #flush at the end of this method, the problem can be
> reproduced. At the time, I was not able to come up with a unit test that would
> reproduce the problem, but my best guess as to what was happening is in the
> update comment:
>
>
>   Name: Collections-dtl.568
>   Author: dtl
>   Time: 5 May 2014, 12:39:30.026 pm
>  
>   Add a flush to WriteStream>>nextChunkPut:
>  
>   This is a workaround for a bug in the runtime library of some versions of
>   Ubuntu. The symptom is that creation of a class comment for a class that
>   previously had no comment leads to a file size error in the new RemoteStream
>   that points to the class comment. Actual file size and contents of the
>   changes file are not affected by this bug, and the error occurs when reading
>   contents of the changes file immediately following the initial save, Flushing
>   the stream after writing a chunk to the changes file prevents the problem.
>
> Dave
>

Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

Levente Uzonyi-2
I checked the code, and came to the conclusion that it's not an OS bug.
The cause of the problem is that we're writing the file (the changes file
in this case) using one file descriptor, and try to read its contents
using other descriptors (the read-only copies of the source files).
But the written bytes will only become visible to other file
descriptors of the same file after calling the fflush() function (which is
what does #flush do).

Here are some snippets showing how it works:

"This one should fail, because the bytes are not flushed."
StandardFileStream newFileNamed: 'test.txt' do: [ :file |
  | readContents |
  file nextPutAll: 'test'.
  readContents := StandardFileStream readOnlyFileNamed: 'test.txt' do: [ :file2 |
  file2 contents ].
  self assert: readContents = 'test' ].

"Sending #flush will make it work."
StandardFileStream newFileNamed: 'test.txt' do: [ :file |
  | readContents |
  file nextPutAll: 'test'; flush.
  readContents := StandardFileStream readOnlyFileNamed: 'test.txt' do: [ :file2 |
  file2 contents ].
  self assert: readContents = 'test' ].

"Reading from the same file descriptor always works."
StandardFileStream newFileNamed: 'test.txt' do: [ :file |
  | readContents |
  file nextPutAll: 'test'.
  readContents := file reset; next: 4.
  self assert: readContents = 'test' ]

The reason why the old code used to work, is because there was only one
file descriptor used to read and write the changes file.

#flush is pretty costly, and IMO it's called way too often if it's in
#nextChunkPut:. The most common workaround to avoid frequent calls is to
use InMidstOfFileinNotification to check if it's a bulk write, and flush
only once in those cases. For some reason this technique is not used in
case of class comments.
I changed the last lines of ClassDescription >> #classComment:stamp: in my
image to

  self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
  InMidstOfFileinNotification signal ifFalse: [file flush].
  SystemChangeNotifier uniqueInstance classCommented: self.

Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems
to me that the problem is gone.

Levente

On Sun, 8 Mar 2015, David T. Lewis wrote:

> On Sun, Mar 08, 2015 at 11:29:18AM -0400, David T. Lewis wrote:
>> On Sun, Mar 08, 2015 at 11:44:18AM +0100, Tobias Pape wrote:
>>> Hi,
>>>
>>> On 08.03.2015, at 04:18, Levente Uzonyi <[hidden email]> wrote:
>>>
>>>> On Sat, 7 Mar 2015, David T. Lewis wrote:
>>>>
>>>>> On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
>>>>>> Hey,
>>>>>> On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
>>>>>>
>>>>>>> The image fixes the WriteStream>>NextChunkPut error we where getting when trying to modify comments which makes me happy (:
>>>>>>
>>>>>> to which fix do you refer?
>>>>>> There is a hack out there calling flush every now and then,
>>>>>> but this all calls for a principled solution for streams that
>>>>>> read _and_ write.
>>>>>
>>>>> You are right that it is a hack, but the actual problem is a defect
>>>>> in some C runtime libraries, notably on Ubuntu but possibly others
>>>>> as well.
>>>>
>>>> Are you sure it's a bug in the OS? Isn't it just the allocate-on-flush behavior?https://en.wikipedia.org/wiki/Allocate-on-flush
>>
>> No, I am not sure. I had convinced myself that it was a libc issue, but I may be wrong.
>>
>
> To check this, I tried running an image from a thumb drive with vfat file
> system, which presumably does not have the allocate-on-flush feature. The
> bug is still present (confirmed by reverting WriteStream>>nextChunkPut: to
> the earlier version). So the problem does not appear to be associated with
> the file system.
>
> Dave
>
>
>
>>>
>>> I too doubt an OS bug.
>>> Someone (i don't remember) said to me, that the behavior of the c calls
>>> make clear not to rely on certain thing.
>>> I plan to investigate the issue soonish.
>>>
>>> best
>>> -tobias
>>
>> That would be great.
>>
>> The workaround that "fixes" the comment problem is in WriteStream>>nextChunkPut:,
>> so if you remove the #flush at the end of this method, the problem can be
>> reproduced. At the time, I was not able to come up with a unit test that would
>> reproduce the problem, but my best guess as to what was happening is in the
>> update comment:
>>
>>
>>   Name: Collections-dtl.568
>>   Author: dtl
>>   Time: 5 May 2014, 12:39:30.026 pm
>>
>>   Add a flush to WriteStream>>nextChunkPut:
>>
>>   This is a workaround for a bug in the runtime library of some versions of
>>   Ubuntu. The symptom is that creation of a class comment for a class that
>>   previously had no comment leads to a file size error in the new RemoteStream
>>   that points to the class comment. Actual file size and contents of the
>>   changes file are not affected by this bug, and the error occurs when reading
>>   contents of the changes file immediately following the initial save, Flushing
>>   the stream after writing a chunk to the changes file prevents the problem.
>>
>> Dave
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

David T. Lewis
On Sun, Mar 08, 2015 at 08:03:54PM +0100, Levente Uzonyi wrote:
> I checked the code, and came to the conclusion that it's not an OS bug.
> The cause of the problem is that we're writing the file (the changes file
> in this case) using one file descriptor, and try to read its contents
> using other descriptors (the read-only copies of the source files).
> But the written bytes will only become visible to other file
> descriptors of the same file after calling the fflush() function (which is
> what does #flush do).

You are right about what is happening, although I'm not sure if it is a
bug in the clib, or if it is really just an invalid assumption on our part
about how shared FILE streams should behave.

I put a test in the inbox that shows the problem. It is very similar to
your snippets below.

The FilePlugin uses fseek() and ftell() to determine file size and position.
Apparently, if the VM process has two FILE streams open on the same file
descriptor, some C runtimes will handle the file position tracking differently
than others. My Ubuntu runtime is different compared to that of my older SuSE
system, or my Windows system. I was assuming that it was a bug, but maybe not.

Your InMidstOfFileinNotification fix does look like a better way to handle
the problem.

Dave

>
> Here are some snippets showing how it works:
>
> "This one should fail, because the bytes are not flushed."
> StandardFileStream newFileNamed: 'test.txt' do: [ :file |
> | readContents |
> file nextPutAll: 'test'.
> readContents := StandardFileStream readOnlyFileNamed: 'test.txt' do:
> [ :file2 |
> file2 contents ].
> self assert: readContents = 'test' ].
>
> "Sending #flush will make it work."
> StandardFileStream newFileNamed: 'test.txt' do: [ :file |
> | readContents |
> file nextPutAll: 'test'; flush.
> readContents := StandardFileStream readOnlyFileNamed: 'test.txt' do:
> [ :file2 |
> file2 contents ].
> self assert: readContents = 'test' ].
>
> "Reading from the same file descriptor always works."
> StandardFileStream newFileNamed: 'test.txt' do: [ :file |
> | readContents |
> file nextPutAll: 'test'.
> readContents := file reset; next: 4.
> self assert: readContents = 'test' ]
>
> The reason why the old code used to work, is because there was only one
> file descriptor used to read and write the changes file.
>
> #flush is pretty costly, and IMO it's called way too often if it's in
> #nextChunkPut:. The most common workaround to avoid frequent calls is to
> use InMidstOfFileinNotification to check if it's a bulk write, and flush
> only once in those cases. For some reason this technique is not used in
> case of class comments.
> I changed the last lines of ClassDescription >> #classComment:stamp: in my
> image to
>
> self organization classComment: (RemoteString newString: aString
> onFileNumber: 2) stamp: aStamp.
> InMidstOfFileinNotification signal ifFalse: [file flush].
> SystemChangeNotifier uniqueInstance classCommented: self.
>
> Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems
> to me that the problem is gone.
>
> Levente
>
> On Sun, 8 Mar 2015, David T. Lewis wrote:
>
> >On Sun, Mar 08, 2015 at 11:29:18AM -0400, David T. Lewis wrote:
> >>On Sun, Mar 08, 2015 at 11:44:18AM +0100, Tobias Pape wrote:
> >>>Hi,
> >>>
> >>>On 08.03.2015, at 04:18, Levente Uzonyi <[hidden email]> wrote:
> >>>
> >>>>On Sat, 7 Mar 2015, David T. Lewis wrote:
> >>>>
> >>>>>On Sat, Mar 07, 2015 at 08:28:55PM +0100, Tobias Pape wrote:
> >>>>>>Hey,
> >>>>>>On 07.03.2015, at 19:01, gettimothy <[hidden email]> wrote:
> >>>>>>
> >>>>>>>The image fixes the WriteStream>>NextChunkPut error we where getting
> >>>>>>>when trying to modify comments which makes me happy (:
> >>>>>>
> >>>>>>to which fix do you refer?
> >>>>>>There is a hack out there calling flush every now and then,
> >>>>>>but this all calls for a principled solution for streams that
> >>>>>>read _and_ write.
> >>>>>
> >>>>>You are right that it is a hack, but the actual problem is a defect
> >>>>>in some C runtime libraries, notably on Ubuntu but possibly others
> >>>>>as well.
> >>>>
> >>>>Are you sure it's a bug in the OS? Isn't it just the allocate-on-flush
> >>>>behavior?https://en.wikipedia.org/wiki/Allocate-on-flush
> >>
> >>No, I am not sure. I had convinced myself that it was a libc issue, but I
> >>may be wrong.
> >>
> >
> >To check this, I tried running an image from a thumb drive with vfat file
> >system, which presumably does not have the allocate-on-flush feature. The
> >bug is still present (confirmed by reverting WriteStream>>nextChunkPut: to
> >the earlier version). So the problem does not appear to be associated with
> >the file system.
> >
> >Dave
> >
> >
> >
> >>>
> >>>I too doubt an OS bug.
> >>>Someone (i don't remember) said to me, that the behavior of the c calls
> >>>make clear not to rely on certain thing.
> >>>I plan to investigate the issue soonish.
> >>>
> >>>best
> >>> -tobias
> >>
> >>That would be great.
> >>
> >>The workaround that "fixes" the comment problem is in
> >>WriteStream>>nextChunkPut:,
> >>so if you remove the #flush at the end of this method, the problem can be
> >>reproduced. At the time, I was not able to come up with a unit test that
> >>would
> >>reproduce the problem, but my best guess as to what was happening is in
> >>the
> >>update comment:
> >>
> >>
> >>  Name: Collections-dtl.568
> >>  Author: dtl
> >>  Time: 5 May 2014, 12:39:30.026 pm
> >>
> >>  Add a flush to WriteStream>>nextChunkPut:
> >>
> >>  This is a workaround for a bug in the runtime library of some versions
> >>  of
> >>  Ubuntu. The symptom is that creation of a class comment for a class that
> >>  previously had no comment leads to a file size error in the new
> >>  RemoteStream
> >>  that points to the class comment. Actual file size and contents of the
> >>  changes file are not affected by this bug, and the error occurs when
> >>  reading
> >>  contents of the changes file immediately following the initial save,
> >>  Flushing
> >>  the stream after writing a chunk to the changes file prevents the
> >>  problem.
> >>
> >>Dave
> >>
> >
> >

Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

Tobias Pape
In reply to this post by Levente Uzonyi-2
Hi,

thanks for taking the time to check!


On 08.03.2015, at 20:03, Levente Uzonyi <[hidden email]> wrote:

>
> #flush is pretty costly, and IMO it's called way too often if it's in
> #nextChunkPut:. The most common workaround to avoid frequent calls is to use InMidstOfFileinNotification to check if it's a bulk write, and flush only once in those cases. For some reason this technique is not used in case of class comments.
> I changed the last lines of ClassDescription >> #classComment:stamp: in my image to
>
> self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
> InMidstOfFileinNotification signal ifFalse: [file flush].
> SystemChangeNotifier uniqueInstance classCommented: self.
>
> Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems to me that the problem is gone.

Can you tell me whether the Changes file is read/written via one stream or two?
I understood its  two file descriptors but are these two in different streams or
the same stream?

Best
        -Tobias


Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

Levente Uzonyi-2
On Sun, 8 Mar 2015, Tobias Pape wrote:

> Hi,
>
> thanks for taking the time to check!
>
>
> On 08.03.2015, at 20:03, Levente Uzonyi <[hidden email]> wrote:
>
>>
>> #flush is pretty costly, and IMO it's called way too often if it's in
>> #nextChunkPut:. The most common workaround to avoid frequent calls is to use InMidstOfFileinNotification to check if it's a bulk write, and flush only once in those cases. For some reason this technique is not used in case of class comments.
>> I changed the last lines of ClassDescription >> #classComment:stamp: in my image to
>>
>> self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
>> InMidstOfFileinNotification signal ifFalse: [file flush].
>> SystemChangeNotifier uniqueInstance classCommented: self.
>>
>> Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems to me that the problem is gone.
>
> Can you tell me whether the Changes file is read/written via one stream or two?
> I understood its  two file descriptors but are these two in different streams or
> the same stream?

Of course, those are two (or more) different streams. We use SourceFiles
at: 2 for writing and CurrentReadOnlySourceFiles at: 2 for reading.

Levente

>
> Best
> -Tobias
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

Tobias Pape

On 08.03.2015, at 20:43, Levente Uzonyi <[hidden email]> wrote:

> On Sun, 8 Mar 2015, Tobias Pape wrote:
>
>> Hi,
>>
>> thanks for taking the time to check!
>>
>>
>> On 08.03.2015, at 20:03, Levente Uzonyi <[hidden email]> wrote:
>>
>>>
>>> #flush is pretty costly, and IMO it's called way too often if it's in
>>> #nextChunkPut:. The most common workaround to avoid frequent calls is to use InMidstOfFileinNotification to check if it's a bulk write, and flush only once in those cases. For some reason this technique is not used in case of class comments.
>>> I changed the last lines of ClassDescription >> #classComment:stamp: in my image to
>>>
>>> self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
>>> InMidstOfFileinNotification signal ifFalse: [file flush].
>>> SystemChangeNotifier uniqueInstance classCommented: self.
>>>
>>> Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems to me that the problem is gone.
>>
>> Can you tell me whether the Changes file is read/written via one stream or two?
>> I understood its  two file descriptors but are these two in different streams or
>> the same stream?
>
> Of course, those are two (or more) different streams. We use SourceFiles at: 2 for writing and CurrentReadOnlySourceFiles at: 2 for reading.

Darn; if it had been a ReadWriteStream, one could say that we'd only really needed #flush when
we switch from writing to reading modeā€¦.
So it is more elaborate.

Best
        -Tobias
Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut: (was: Squeak 4.6 release update)

David T. Lewis
On Sun, Mar 08, 2015 at 08:49:40PM +0100, Tobias Pape wrote:

>
> On 08.03.2015, at 20:43, Levente Uzonyi <[hidden email]> wrote:
>
> > On Sun, 8 Mar 2015, Tobias Pape wrote:
> >
> >> Hi,
> >>
> >> thanks for taking the time to check!
> >>
> >>
> >> On 08.03.2015, at 20:03, Levente Uzonyi <[hidden email]> wrote:
> >>
> >>>
> >>> #flush is pretty costly, and IMO it's called way too often if it's in
> >>> #nextChunkPut:. The most common workaround to avoid frequent calls is to use InMidstOfFileinNotification to check if it's a bulk write, and flush only once in those cases. For some reason this technique is not used in case of class comments.
> >>> I changed the last lines of ClassDescription >> #classComment:stamp: in my image to
> >>>
> >>> self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
> >>> InMidstOfFileinNotification signal ifFalse: [file flush].
> >>> SystemChangeNotifier uniqueInstance classCommented: self.
> >>>
> >>> Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems to me that the problem is gone.
> >>
> >> Can you tell me whether the Changes file is read/written via one stream or two?
> >> I understood its  two file descriptors but are these two in different streams or
> >> the same stream?
> >
> > Of course, those are two (or more) different streams. We use SourceFiles at: 2 for writing and CurrentReadOnlySourceFiles at: 2 for reading.
>
> Darn; if it had been a ReadWriteStream, one could say that we'd only really needed #flush when
> we switch from writing to reading mode?.
> So it is more elaborate.
>
I am attaching a small C program that shows the problem. This uses the
same kind of fseek/ftell calls used in the VM FilePlugin. This confirms
the different C runtime behaviour. On my Ubuntu system, an explicit
flush is required, and on my SuSE box, no flush is required.

Levente is probably right in saying that this is not an OS bug. It may
just be different behaviour in different versions of libc.

Dave





teststreams.c (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut:

Andreas Wacknitz
FWIW, Solaris 10 (SPARC), OpenSolaris (SPARC) and openindiana (x64) print the same as your Ubuntu installation does.

Regards,
Andreas

Am 08.03.15 21:38, schrieb David T. Lewis:
On Sun, Mar 08, 2015 at 08:49:40PM +0100, Tobias Pape wrote:
On 08.03.2015, at 20:43, Levente Uzonyi [hidden email] wrote:

On Sun, 8 Mar 2015, Tobias Pape wrote:

Hi,

thanks for taking the time to check!


On 08.03.2015, at 20:03, Levente Uzonyi [hidden email] wrote:

#flush is pretty costly, and IMO it's called way too often if it's in
#nextChunkPut:. The most common workaround to avoid frequent calls is to use InMidstOfFileinNotification to check if it's a bulk write, and flush only once in those cases. For some reason this technique is not used in case of class comments.
I changed the last lines of ClassDescription >> #classComment:stamp: in my image to

	self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
	InMidstOfFileinNotification signal ifFalse: [file flush].
	SystemChangeNotifier uniqueInstance classCommented: self.

Then removed the #flush from WriteStream >> #nextChunkPut:, and it seems to me that the problem is gone.
Can you tell me whether the Changes file is read/written via one stream or two?
I understood its  two file descriptors but are these two in different streams or
the same stream?
Of course, those are two (or more) different streams. We use SourceFiles at: 2 for writing and CurrentReadOnlySourceFiles at: 2 for reading.
Darn; if it had been a ReadWriteStream, one could say that we'd only really needed #flush when
we switch from writing to reading mode?.
So it is more elaborate.

I am attaching a small C program that shows the problem. This uses the
same kind of fseek/ftell calls used in the VM FilePlugin. This confirms
the different C runtime behaviour. On my Ubuntu system, an explicit
flush is required, and on my SuSE box, no flush is required.

Levente is probably right in saying that this is not an OS bug. It may
just be different behaviour in different versions of libc.

Dave





    



Reply | Threaded
Open this post in threaded view
|

Re: Reassessing the hack in WriteStream>>nextChunkPut:

David T. Lewis
On Sun, Mar 08, 2015 at 10:39:32PM +0100, Andreas Wacknitz wrote:

> Am 08.03.15 21:38, schrieb David T. Lewis:
> >On Sun, Mar 08, 2015 at 08:49:40PM +0100, Tobias Pape wrote:
> >>On 08.03.2015, at 20:43, Levente Uzonyi <[hidden email]> wrote:
> >>>On Sun, 8 Mar 2015, Tobias Pape wrote:
> >>>>
> >>>>thanks for taking the time to check!
> >>>>
> >>>>On 08.03.2015, at 20:03, Levente Uzonyi <[hidden email]> wrote:
> >>>>
> >>>>>#flush is pretty costly, and IMO it's called way too often if it's in
> >>>>>#nextChunkPut:. The most common workaround to avoid frequent calls is
> >>>>>to use InMidstOfFileinNotification to check if it's a bulk write, and
> >>>>>flush only once in those cases. For some reason this technique is not
> >>>>>used in case of class comments.
> >>>>>I changed the last lines of ClassDescription >> #classComment:stamp:
> >>>>>in my image to
> >>>>>
> >>>>> self organization classComment: (RemoteString newString: aString
> >>>>> onFileNumber: 2) stamp: aStamp.
> >>>>> InMidstOfFileinNotification signal ifFalse: [file flush].
> >>>>> SystemChangeNotifier uniqueInstance classCommented: self.
> >>>>>
> >>>>>Then removed the #flush from WriteStream >> #nextChunkPut:, and it
> >>>>>seems to me that the problem is gone.
> >>>>Can you tell me whether the Changes file is read/written via one stream
> >>>>or two?
> >>>>I understood its  two file descriptors but are these two in different
> >>>>streams or
> >>>>the same stream?
> >>>Of course, those are two (or more) different streams. We use SourceFiles
> >>>at: 2 for writing and CurrentReadOnlySourceFiles at: 2 for reading.
> >>Darn; if it had been a ReadWriteStream, one could say that we'd only
> >>really needed #flush when
> >>we switch from writing to reading mode?.
> >>So it is more elaborate.
> >>
> >I am attaching a small C program that shows the problem. This uses the
> >same kind of fseek/ftell calls used in the VM FilePlugin. This confirms
> >the different C runtime behaviour. On my Ubuntu system, an explicit
> >flush is required, and on my SuSE box, no flush is required.
> >
> >Levente is probably right in saying that this is not an OS bug. It may
> >just be different behaviour in different versions of libc.
>
> FWIW, Solaris 10 (SPARC), OpenSolaris (SPARC) and openindiana (x64)
> print the same as your Ubuntu installation does.

Thanks Andreas,

This confirms that it is not an OS bug (my apologies to Ubuntu).

I also tried Levente's change to ClassDescription>>classComment:stamp:
to use InMidstOfFileinNotification, and reverted WriteStream>>nextChunkPut:
back to the original version (yo 8/13/2003) to remove the flush. It works
fine on my Ubuntu system.

I think Levente's fix is the right thing to do. Levente, do you want
to commit the change?

Dave





Reply | Threaded
Open this post in threaded view
|

Re: Squeak 4.6 release update

marcel.taeumel (old)
In reply to this post by Chris Muller-4
Nice!

Currently, I am finishing tree search for the Object Explorer (i.e. all pluggable tree morphs). I will commit it soonerish. :-)

Best,
Marcel