The Inbox: System-ct.1136.mcz

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

The Inbox: System-ct.1136.mcz

commits-2
Christoph Thiede uploaded a new version of System to project The Inbox:
http://source.squeak.org/inbox/System-ct.1136.mcz

==================== Summary ====================

Name: System-ct.1136
Author: ct
Time: 6 February 2020, 7:50:02.954648 pm
UUID: 0a637557-d32d-9347-bc4b-4f2bb95a707d
Ancestors: System-cmm.1131

Make Smalltalk more robust against interruptions/timeouts during sources compilation

A popular example is a test case that compiles a lot of methods and then is terminated via timeout. See [1] for an example. This commit ensures that the changes file does not remain closed when the method is curtailed.

Before probably destroyed your image, now works:

        [MCSnapshotResource mockPackage unload]
                valueWithin: 1 seconds "you may need to adapt the limit to reproduce"
                onTimeout: [].

[1] http://forum.world.st/BUG-MultiByteFileStream-Object-gt-gt-error-primGetPosition-failed-td5111181.html

=============== Diff against System-cmm.1131 ===============

Item was changed:
  ----- Method: SmalltalkImage>>forceChangesToDisk (in category 'sources, changes log') -----
  forceChangesToDisk
  "Ensure that the changes file has been fully written to disk by closing and re-opening it. This makes the system more robust in the face of a power failure or hard-reboot."
 
  | changesFile |
  changesFile := SourceFiles at: 2.
  (changesFile isKindOf: FileStream) ifTrue: [
  changesFile flush.
+ SecurityManager default hasFileAccess ifTrue: [
+ [changesFile close] ensure: [
+ changesFile open: changesFile name forWrite: true]].
+ changesFile setToEnd].!
- SecurityManager default hasFileAccess ifTrue:[
- changesFile close.
- changesFile open: changesFile name forWrite: true].
- changesFile setToEnd.
- ].
- !


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1136.mcz

Levente Uzonyi
On Thu, 6 Feb 2020, [hidden email] wrote:

> Christoph Thiede uploaded a new version of System to project The Inbox:
> http://source.squeak.org/inbox/System-ct.1136.mcz
>
> ==================== Summary ====================
>
> Name: System-ct.1136
> Author: ct
> Time: 6 February 2020, 7:50:02.954648 pm
> UUID: 0a637557-d32d-9347-bc4b-4f2bb95a707d
> Ancestors: System-cmm.1131
>
> Make Smalltalk more robust against interruptions/timeouts during sources compilation
>
> A popular example is a test case that compiles a lot of methods and then is terminated via timeout. See [1] for an example. This commit ensures that the changes file does not remain closed when the method is curtailed.
>
> Before probably destroyed your image, now works:
>
> [MCSnapshotResource mockPackage unload]
> valueWithin: 1 seconds "you may need to adapt the limit to reproduce"
> onTimeout: [].
>
> [1] http://forum.world.st/BUG-MultiByteFileStream-Object-gt-gt-error-primGetPosition-failed-td5111181.html
>
> =============== Diff against System-cmm.1131 ===============
>
> Item was changed:
>  ----- Method: SmalltalkImage>>forceChangesToDisk (in category 'sources, changes log') -----
>  forceChangesToDisk
>   "Ensure that the changes file has been fully written to disk by closing and re-opening it. This makes the system more robust in the face of a power failure or hard-reboot."
>
>   | changesFile |
>   changesFile := SourceFiles at: 2.
>   (changesFile isKindOf: FileStream) ifTrue: [
>   changesFile flush.
> + SecurityManager default hasFileAccess ifTrue: [
> + [changesFile close] ensure: [
> + changesFile open: changesFile name forWrite: true]].
> + changesFile setToEnd].!

I don't think today's operating systems write changes to disk if you
reopen the file.
#sync is probably the right method here, though it may slow things
down based on how often #forceChangesToDisk is sent.

Levente

> - SecurityManager default hasFileAccess ifTrue:[
> - changesFile close.
> - changesFile open: changesFile name forWrite: true].
> - changesFile setToEnd.
> - ].
> - !

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1136.mcz

Christoph Thiede

Hi Levente,


for a start, my only objective was to increase the robustness. Do you think #ensure: is an appropriate approach for that? :)


I don't think today's operating systems write changes to disk if you reopen the file.

Could you explain this? :) If the file is closed, the OS cannot know we want to reopen it after. So it must save (or reject) any changes and release the resource so that any other applications are enabled to access it. What's wrong with this understanding? And how can #sync (= flush) be slower than reopening, which requires flushing as well?

By the way, it appears that there are many, many calls to #forceChangesToDisk while loading/unloading a Monticello snapshot ... Should we maybe use something like #cacheDuring: for this operation?

Best,
Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von Levente Uzonyi <[hidden email]>
Gesendet: Freitag, 7. Februar 2020 10:52:03
An: [hidden email]
Betreff: Re: [squeak-dev] The Inbox: System-ct.1136.mcz
 
On Thu, 6 Feb 2020, [hidden email] wrote:

> Christoph Thiede uploaded a new version of System to project The Inbox:
> http://source.squeak.org/inbox/System-ct.1136.mcz
>
> ==================== Summary ====================
>
> Name: System-ct.1136
> Author: ct
> Time: 6 February 2020, 7:50:02.954648 pm
> UUID: 0a637557-d32d-9347-bc4b-4f2bb95a707d
> Ancestors: System-cmm.1131
>
> Make Smalltalk more robust against interruptions/timeouts during sources compilation
>
> A popular example is a test case that compiles a lot of methods and then is terminated via timeout. See [1] for an example. This commit ensures that the changes file does not remain closed when the method is curtailed.
>
> Before probably destroyed your image, now works:
>
>        [MCSnapshotResource mockPackage unload]
>                valueWithin: 1 seconds "you may need to adapt the limit to reproduce"
>                onTimeout: [].
>
> [1] http://forum.world.st/BUG-MultiByteFileStream-Object-gt-gt-error-primGetPosition-failed-td5111181.html
>
> =============== Diff against System-cmm.1131 ===============
>
> Item was changed:
>  ----- Method: SmalltalkImage>>forceChangesToDisk (in category 'sources, changes log') -----
>  forceChangesToDisk
>        "Ensure that the changes file has been fully written to disk by closing and re-opening it. This makes the system more robust in the face of a power failure or hard-reboot."
>
>        | changesFile |
>        changesFile := SourceFiles at: 2.
>        (changesFile isKindOf: FileStream) ifTrue: [
>                changesFile flush.
> +              SecurityManager default hasFileAccess ifTrue: [
> +                      [changesFile close] ensure: [
> +                              changesFile open: changesFile name forWrite: true]].
> +              changesFile setToEnd].!

I don't think today's operating systems write changes to disk if you
reopen the file.
#sync is probably the right method here, though it may slow things
down based on how often #forceChangesToDisk is sent.

Levente

> -              SecurityManager default hasFileAccess ifTrue:[
> -                      changesFile close.
> -                      changesFile open: changesFile name forWrite: true].
> -              changesFile setToEnd.
> -      ].
> - !



Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1136.mcz

Levente Uzonyi
Hi Christoph,

On Fri, 7 Feb 2020, Thiede, Christoph wrote:

>
> Hi Levente,
>
>
> for a start, my only objective was to increase the robustness. Do you think #ensure: is an appropriate approach for that? :)

Sure.

>
>
> > I don't think today's operating systems write changes to disk if you reopen the file.
> Could you explain this? :) If the file is closed, the OS cannot know we want to reopen it after. So it must save (or reject) any changes and release the resource so that any other applications are enabled to access it. What's
> wrong with this understanding? And how can #sync (= flush) be slower than reopening, which requires flushing as well?

#sync is not flush. #sync sends #flush and calls fsync(). The latter
tells the OS to write its buffers related to the given file to the disk.
#flush calls fflush(), which tells the OS to synchronize the processes's
buffers of the given file with the OS's buffers, so that all processes see
the changes you've done.
When you close a file, the OS notes that file is closed, but actual disk
writes may, and usually will be delayed to improve performance. So, if
you reopen a file, the file's contents will usually be written to the disk at
some later point in time.

You can run a simple benchmark to see what your OS does:

filename := UUID new asString36.
"create the file"
FileDirectory default newFileNamed: filename do: [ :file | "nothing" ].
results := #(flush reopen sync) collect: [ :each |
  each -> (FileDirectory default
  fileNamed: filename
  do: [ :file |
  file truncate.
  [ file nextPut: $1; perform: each ] benchFor: 1 seconds ]) ].
FileDirectory default deleteFileNamed: filename.
results.

My result are
{
  #flush->'650,000 per second. 1.54 microseconds per run.' .
  #reopen->'94,700 per second. 10.6 microseconds per run.' .
  #sync->'202 per second. 4.94 milliseconds per run.'
}


>
> By the way, it appears that there are many, many calls to #forceChangesToDisk while loading/unloading a Monticello snapshot ... Should we maybe use something like #cacheDuring: for this operation?

cacheDuring: is a method of CurrentReadOnlySourceFiles. That doesn't help
with writes. Something similar could be done to avoid trashing your disk
though.


Levente

>
> Best,
> Christoph
>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <[hidden email]> im Auftrag von Levente Uzonyi <[hidden email]>
> Gesendet: Freitag, 7. Februar 2020 10:52:03
> An: [hidden email]
> Betreff: Re: [squeak-dev] The Inbox: System-ct.1136.mcz  
> On Thu, 6 Feb 2020, [hidden email] wrote:
>
> > Christoph Thiede uploaded a new version of System to project The Inbox:
> > http://source.squeak.org/inbox/System-ct.1136.mcz
> >
> > ==================== Summary ====================
> >
> > Name: System-ct.1136
> > Author: ct
> > Time: 6 February 2020, 7:50:02.954648 pm
> > UUID: 0a637557-d32d-9347-bc4b-4f2bb95a707d
> > Ancestors: System-cmm.1131
> >
> > Make Smalltalk more robust against interruptions/timeouts during sources compilation
> >
> > A popular example is a test case that compiles a lot of methods and then is terminated via timeout. See [1] for an example. This commit ensures that the changes file does not remain closed when the method is curtailed.
> >
> > Before probably destroyed your image, now works:
> >
> >        [MCSnapshotResource mockPackage unload]
> >                valueWithin: 1 seconds "you may need to adapt the limit to reproduce"
> >                onTimeout: [].
> >
> > [1] http://forum.world.st/BUG-MultiByteFileStream-Object-gt-gt-error-primGetPosition-failed-td5111181.html
> >
> > =============== Diff against System-cmm.1131 ===============
> >
> > Item was changed:
> >  ----- Method: SmalltalkImage>>forceChangesToDisk (in category 'sources, changes log') -----
> >  forceChangesToDisk
> >        "Ensure that the changes file has been fully written to disk by closing and re-opening it. This makes the system more robust in the face of a power failure or hard-reboot."
> >
> >        | changesFile |
> >        changesFile := SourceFiles at: 2.
> >        (changesFile isKindOf: FileStream) ifTrue: [
> >                changesFile flush.
> > +              SecurityManager default hasFileAccess ifTrue: [
> > +                      [changesFile close] ensure: [
> > +                              changesFile open: changesFile name forWrite: true]].
> > +              changesFile setToEnd].!
>
> I don't think today's operating systems write changes to disk if you
> reopen the file.
> #sync is probably the right method here, though it may slow things
> down based on how often #forceChangesToDisk is sent.
>
> Levente
>
> > -              SecurityManager default hasFileAccess ifTrue:[
> > -                      changesFile close.
> > -                      changesFile open: changesFile name forWrite: true].
> > -              changesFile setToEnd.
> > -      ].
> > - !
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1136.mcz

David T. Lewis
In reply to this post by Levente Uzonyi
On Fri, Feb 07, 2020 at 10:52:03AM +0100, Levente Uzonyi wrote:

> On Thu, 6 Feb 2020, [hidden email] wrote:
>
> >Christoph Thiede uploaded a new version of System to project The Inbox:
> >http://source.squeak.org/inbox/System-ct.1136.mcz
> >
> >==================== Summary ====================
> >
> >Name: System-ct.1136
> >Author: ct
> >Time: 6 February 2020, 7:50:02.954648 pm
> >UUID: 0a637557-d32d-9347-bc4b-4f2bb95a707d
> >Ancestors: System-cmm.1131
> >
> >Make Smalltalk more robust against interruptions/timeouts during sources
> >compilation
> >
> >A popular example is a test case that compiles a lot of methods and then
> >is terminated via timeout. See [1] for an example. This commit ensures
> >that the changes file does not remain closed when the method is curtailed.
> >
> >Before probably destroyed your image, now works:
> >
> > [MCSnapshotResource mockPackage unload]
> > valueWithin: 1 seconds "you may need to adapt the limit to
> > reproduce"
> > onTimeout: [].
> >
> >[1]
> >http://forum.world.st/BUG-MultiByteFileStream-Object-gt-gt-error-primGetPosition-failed-td5111181.html
> >
> >=============== Diff against System-cmm.1131 ===============
> >
> >Item was changed:
> > ----- Method: SmalltalkImage>>forceChangesToDisk (in category 'sources,
> > changes log') -----
> > forceChangesToDisk
> > "Ensure that the changes file has been fully written to disk by
> > closing and re-opening it. This makes the system more robust in the
> > face of a power failure or hard-reboot."
> >
> > | changesFile |
> > changesFile := SourceFiles at: 2.
> > (changesFile isKindOf: FileStream) ifTrue: [
> > changesFile flush.
> >+ SecurityManager default hasFileAccess ifTrue: [
> >+ [changesFile close] ensure: [
> >+ changesFile open: changesFile name forWrite:
> >true]].
> >+ changesFile setToEnd].!
>
> I don't think today's operating systems write changes to disk if you
> reopen the file.
> #sync is probably the right method here, though it may slow things
> down based on how often #forceChangesToDisk is sent.
>

Better to try it with flush rather than sync. On Windows, sqFileSync() just
calls sqFileFlush(() anyway, and the flush does FlushFileBuffers().

Dave


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1136.mcz

Levente Uzonyi
On Fri, 7 Feb 2020, David T. Lewis wrote:

>
> Better to try it with flush rather than sync. On Windows, sqFileSync() just
> calls sqFileFlush(() anyway, and the flush does FlushFileBuffers().

Right. It seems to be not possible to force changes to the physical disk
on windows. And flush technically does the same as reopen on these
platforms.
So yes, flush is the better choice with an additional comment about the
method not really forcing those files to the disk.

Levente

>
> Dave