About SQFile's fileSize field

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

About SQFile's fileSize field

Levente Uzonyi
 
Hi All,

The SQFile struct[1] has a field named fileSize, which is used to cache[2]
the size of the file when such property makes sense.
This caching behavior seems unnecessary, and it causes synchronization
problems between multiple SQFile structures of a single file. The latter
could be worked around by updating the cached value more often, but I was
wondering why is the value cached at all. Does anyone know?

Levente

[1] https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/FilePlugin.h
[2] https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c
Reply | Threaded
Open this post in threaded view
|

Re: About SQFile's fileSize field

Levente Uzonyi
 
I went ahead a bit, removed the field, updated the sources, and compiled a
VM with the changes. It works fine on linux. I've updated the windows and
mac files too, but I can't test those.
There are some remaining references to the removed field in the RiscOS
platforms file, but I don't really know what to do with that. The code
seems to be using platform specific calls to implement the plugin, which
is unusual, but may be more efficient than the generic C implementation.

Anyway, the code is on github[1]. I'll create a pull request once someone
says something about the idea.

Levente

[1] https://github.com/OpenSmalltalk/opensmalltalk-vm/compare/Cog...smalltalking:Cog

On Tue, 10 Oct 2017, Levente Uzonyi wrote:

>
> Hi All,
>
> The SQFile struct[1] has a field named fileSize, which is used to cache[2]
> the size of the file when such property makes sense.
> This caching behavior seems unnecessary, and it causes synchronization
> problems between multiple SQFile structures of a single file. The latter
> could be worked around by updating the cached value more often, but I was
> wondering why is the value cached at all. Does anyone know?
>
> Levente
>
> [1]
> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/FilePlugin.h
> [2]
> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c
>
Reply | Threaded
Open this post in threaded view
|

Re: About SQFile's fileSize field

timrowledge
 

> On 11-10-2017, at 1:40 PM, Levente Uzonyi <[hidden email]> wrote:
>
> I went ahead a bit, removed the field, updated the sources, and compiled a VM with the changes. It works fine on linux. I've updated the windows and mac files too, but I can't test those.
> There are some remaining references to the removed field in the RiscOS platforms file, but I don't really know what to do with that. The code seems to be using platform specific calls to implement the plugin, which is unusual, but may be more efficient than the generic C implementation.

It’s been a while since I had to write that code but it was a complete replacement for the plugin C code. Part of the problem is/was the use of separate position and read/write steps that I found could happen interleaved in some places thus writing complete nonsene into changes files etc. Part of it was the copying of file IDs and expecting that you could always read and write properly. So I had to implement an entire layer above the RISC OS file handling to track file opening/closing and … etc etc. The good news is nobody (and surprisingly quite a few people used Squeak on RISC OS) ever found a single bug.

Anyway, more to the point, I vaguely recall that I had to (ab)use the size field for something or other.

Ah, nope, it was the lastOp field according to the comments in the C code. Wow, comments in C code - who’d ever expect that?


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: DNPG: Do Not Pass Go


Reply | Threaded
Open this post in threaded view
|

Re: About SQFile's fileSize field

johnmci
 
And the only comment I ever got was the cached value confused some Squeak app that someone wrote so they make their own VM to pull the file size from the file system on each request

Sent from my iPhone

> On Oct 11, 2017, at 17:00, tim Rowledge <[hidden email]> wrote:
>
>
>
>> On 11-10-2017, at 1:40 PM, Levente Uzonyi <[hidden email]> wrote:
>>
>> I went ahead a bit, removed the field, updated the sources, and compiled a VM with the changes. It works fine on linux. I've updated the windows and mac files too, but I can't test those.
>> There are some remaining references to the removed field in the RiscOS platforms file, but I don't really know what to do with that. The code seems to be using platform specific calls to implement the plugin, which is unusual, but may be more efficient than the generic C implementation.
>
> It’s been a while since I had to write that code but it was a complete replacement for the plugin C code. Part of the problem is/was the use of separate position and read/write steps that I found could happen interleaved in some places thus writing complete nonsene into changes files etc. Part of it was the copying of file IDs and expecting that you could always read and write properly. So I had to implement an entire layer above the RISC OS file handling to track file opening/closing and … etc etc. The good news is nobody (and surprisingly quite a few people used Squeak on RISC OS) ever found a single bug.
>
> Anyway, more to the point, I vaguely recall that I had to (ab)use the size field for something or other.
>
> Ah, nope, it was the lastOp field according to the comments in the C code. Wow, comments in C code - who’d ever expect that?
>
>
> tim
> --
> tim Rowledge; [hidden email]; http://www.rowledge.org/tim
> Strange OpCodes: DNPG: Do Not Pass Go
>
>
Reply | Threaded
Open this post in threaded view
|

Re: About SQFile's fileSize field

David T. Lewis
In reply to this post by Levente Uzonyi
 
Just FYI, the data structure is used in OSProcessPlugin also, so changes
may be needed there too. That would be in Slang, not platform sources.

Dave

>
> I went ahead a bit, removed the field, updated the sources, and compiled a
> VM with the changes. It works fine on linux. I've updated the windows and
> mac files too, but I can't test those.
> There are some remaining references to the removed field in the RiscOS
> platforms file, but I don't really know what to do with that. The code
> seems to be using platform specific calls to implement the plugin, which
> is unusual, but may be more efficient than the generic C implementation.
>
> Anyway, the code is on github[1]. I'll create a pull request once someone
> says something about the idea.
>
> Levente
>
> [1]
> https://github.com/OpenSmalltalk/opensmalltalk-vm/compare/Cog...smalltalking:Cog
>
> On Tue, 10 Oct 2017, Levente Uzonyi wrote:
>
>>
>> Hi All,
>>
>> The SQFile struct[1] has a field named fileSize, which is used to
>> cache[2]
>> the size of the file when such property makes sense.
>> This caching behavior seems unnecessary, and it causes synchronization
>> problems between multiple SQFile structures of a single file. The latter
>> could be worked around by updating the cached value more often, but I
>> was
>> wondering why is the value cached at all. Does anyone know?
>>
>> Levente
>>
>> [1]
>> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/FilePlugin.h
>> [2]
>> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: About SQFile's fileSize field

Levente Uzonyi
In reply to this post by johnmci
 
That's exactly what I'm trying to remove here, the cached size field,
because it won't be updated when a file is opened more than once from
the image. And that's not a rare case, because that's how the .changes
file are used.

Levente

On Wed, 11 Oct 2017, John McIntosh wrote:

>
> And the only comment I ever got was the cached value confused some Squeak app that someone wrote so they make their own VM to pull the file size from the file system on each request
>
> Sent from my iPhone
>
>> On Oct 11, 2017, at 17:00, tim Rowledge <[hidden email]> wrote:
>>
>>
>>
>>> On 11-10-2017, at 1:40 PM, Levente Uzonyi <[hidden email]> wrote:
>>>
>>> I went ahead a bit, removed the field, updated the sources, and compiled a VM with the changes. It works fine on linux. I've updated the windows and mac files too, but I can't test those.
>>> There are some remaining references to the removed field in the RiscOS platforms file, but I don't really know what to do with that. The code seems to be using platform specific calls to implement the plugin, which is unusual, but may be more efficient than the generic C implementation.
>>
>> It’s been a while since I had to write that code but it was a complete replacement for the plugin C code. Part of the problem is/was the use of separate position and read/write steps that I found could happen interleaved in some places thus writing complete nonsene into changes files etc. Part of it was the copying of file IDs and expecting that you could always read and write properly. So I had to implement an entire layer above the RISC OS file handling to track file opening/closing and … etc etc. The good news is nobody (and surprisingly quite a few people used Squeak on RISC OS) ever found a single bug.
>>
>> Anyway, more to the point, I vaguely recall that I had to (ab)use the size field for something or other.
>>
>> Ah, nope, it was the lastOp field according to the comments in the C code. Wow, comments in C code - who’d ever expect that?
>>
>>
>> tim
>> --
>> tim Rowledge; [hidden email]; http://www.rowledge.org/tim
>> Strange OpCodes: DNPG: Do Not Pass Go
>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: About SQFile's fileSize field

David T. Lewis
In reply to this post by David T. Lewis
 
Following up on my own post, I just did a quick check of OSProcessPlugin
and I do not think that any changes are needed there, so no worries :-)

Dave

On Wed, Oct 11, 2017 at 05:34:29PM -0400, David T. Lewis wrote:

>  
> Just FYI, the data structure is used in OSProcessPlugin also, so changes
> may be needed there too. That would be in Slang, not platform sources.
>
> Dave
>
> >
> > I went ahead a bit, removed the field, updated the sources, and compiled a
> > VM with the changes. It works fine on linux. I've updated the windows and
> > mac files too, but I can't test those.
> > There are some remaining references to the removed field in the RiscOS
> > platforms file, but I don't really know what to do with that. The code
> > seems to be using platform specific calls to implement the plugin, which
> > is unusual, but may be more efficient than the generic C implementation.
> >
> > Anyway, the code is on github[1]. I'll create a pull request once someone
> > says something about the idea.
> >
> > Levente
> >
> > [1]
> > https://github.com/OpenSmalltalk/opensmalltalk-vm/compare/Cog...smalltalking:Cog
> >
> > On Tue, 10 Oct 2017, Levente Uzonyi wrote:
> >
> >>
> >> Hi All,
> >>
> >> The SQFile struct[1] has a field named fileSize, which is used to
> >> cache[2]
> >> the size of the file when such property makes sense.
> >> This caching behavior seems unnecessary, and it causes synchronization
> >> problems between multiple SQFile structures of a single file. The latter
> >> could be worked around by updating the cached value more often, but I
> >> was
> >> wondering why is the value cached at all. Does anyone know?
> >>
> >> Levente
> >>
> >> [1]
> >> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/FilePlugin.h
> >> [2]
> >> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c
> >>
> >
>
>