[OpenSmalltalk/opensmalltalk-vm] Do not cache fileSize in SQFile (#224)

 

If you have more than one reference to the same file from the image (e.g. the changes file), then the file size cache will prevent changes written by one reference to be noticed by another, so you have to reopen all other references to actualize their cached file size.
These changes will make that unnecessary.

I could only test the code on linux, but hopefully the MacOS/iOS and win32 implementations are correct too.
The changes do not cover platforms Plan9 and RiscOS. That's up to someone who has access to and experience with them.
There's room for optimization on platforms which support the fstat() function (the current implementation just seeks to the end of the file and then back to the original position).

---

You can view, comment on, or merge this pull request online at:

  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224

Commit Summary

• Do not cache fileSize in SQFile
• Merge branch 'Cog' of https://github.com/smalltalking/opensmalltalk-vm into Cog
• Removed further references to fileSize
• Merge branch 'Cog' of https://github.com/OpenSmalltalk/opensmalltalk-vm into Cog

File Changes

• M platforms/Cross/plugins/FilePlugin/FilePlugin.h (1)
• M platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c (45)
• M platforms/Mac OS/vm/sqMacNSPlugin.c (10)
• M platforms/win32/plugins/FilePlugin/sqWin32FilePrims.c (4)

Patch Links:

• https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224.patch
• https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"Do not cache fileSize in SQFile (#224)"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224"}}}</script>

 

Hi @smalltalking,

I'm just wondering why you haven't followed up on this PR?

The linux builds failed because there is still a reference to setSize in
sqFilePluginBasicPrims.c.

When I tested the speed difference, not caching the size added about 2
microseconds to retrieving the file size on my 4 or 5 year old Dell
XPS13 (SSD).

Thanks,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@akgrant43 in #224: Hi @smalltalking,\r\n\r\nI'm just wondering why you haven't followed up on this PR?\r\n\r\nThe linux builds failed because there is still a reference to setSize in\r\nsqFilePluginBasicPrims.c.\r\n\r\nWhen I tested the speed difference, not caching the size added about 2\r\nmicroseconds to retrieving the file size on my 4 or 5 year old Dell\r\nXPS13 (SSD).\r\n\r\nThanks,\r\nAlistair"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224#issuecomment-373102200"}}}</script>

 

Hi @akgrant43,

I wrote these changes last October. There was a small discussion about them on the mailing list.
I wanted to create a pull request in October, but I had some other things to do.
After I made the pull request, I saw that the changes interfered with your FilePlugin changes, so I decided to wait till those are fully integrated.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@smalltalking in #224: Hi @akgrant43,\r\n\r\nI wrote these changes last October. There was a small discussion about them [on the mailing list](http://forum.world.st/About-SQFile-s-fileSize-field-td4981131.html).\r\nI wanted to create a pull request in October, but I had some other things to do.\r\nAfter I made the pull request, I saw that the changes interfered with your FilePlugin changes, so I decided to wait till those are fully integrated.\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224#issuecomment-373184930"}}}</script>

 

Thanks for the explanation and link to the email discussion.

The remaining changes I have planned for FilePlugin will take quite a while (I need to learn about the simulator first).

Can you update the PR and I'll merge it in?

Thanks,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@akgrant43 in #224: Thanks for the explanation and link to the email discussion.\r\n\r\nThe remaining changes I have planned for FilePlugin will take quite a while (I need to learn about the simulator first).\r\n\r\nCan you update the PR and I'll merge it in?\r\n\r\nThanks,\r\nAlistair"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224#issuecomment-373275682"}}}</script>

 

@smalltalking pushed 2 commits.

• cb4e6ab Removed the last remaining setSize call
• 2c8416d Merge branch 'Cog' of https://github.com/OpenSmalltalk/opensmalltalk-vm into Cog

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@smalltalking pushed 2 commits in #224"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224/files/d22bff067263f806fbe011a2255309bc49e4a0e9..2c8416d4c20f3490f34c11bb777a278075036b57"}}}</script>

 

Merged #224.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"Merged #224."}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/224#event-1523797306"}}}</script>