Smalltalk › Squeak › Squeak - Dev

Questions about external objects

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

24 messages Options

Levente Uzonyi-2

Questions about external objects

Hi,

I found that the handling of external objects is suboptimal in the image,
and I came up with some ideas to make it better. I have a working solution
in my image, but it can be even better.

It seems to me that only Semaphores are stored in the external objects
array (slot 39 in the special objects array), but in practice any object
can be stored there. Does it make sense to support objects of any other
kind? If not, then the code could be simplified.

I also thought that I'll create a special semaphore (ExternalSemaphore -
which is a subclass of Semaphore) that knows its own index in the
external objects array. This could be really handy for finding the
object in the array, but the VM is not willing to signal these semaphores
in the external objects array. I thought that this is because the VM
doesn't know anything about this class.
So I tried to change Semaphore itself, but after the change the VM refused
to signal any semaphores in the external objects array. Changing the class
back to the original didn't help either. Without looking at the VM
code, I assumed that the VM will signal anything which has the
class in the 19th slot of the special objects array, but that doesn't seem
to be the case.
Is there any way to make this (add an instatnce variable to Semaphore and
still get signals from the VM, or use instances of a subclass of Semaphore
there) work?

Levente

Bert Freudenberg

Re: Questions about external objects

On 12.10.2014, at 08:04, Levente Uzonyi <[hidden email]> wrote:

> Hi,
>
> I found that the handling of external objects is suboptimal in the image, and I came up with some ideas to make it better. I have a working solution in my image, but it can be even better.
>
> It seems to me that only Semaphores are stored in the external objects array (slot 39 in the special objects array), but in practice any object can be stored there. Does it make sense to support objects of any other kind? If not, then the code could be simplified.

Currently the VM uses the ExternalObjectsArray only for semaphores. Other objects use different mechanisms (e.g. SurfacePlugin for external forms). I'm not sure if there is a good use case for storing non-semaphores in ExternalObjectsArray, maybe ask on the vm-dev list?

> I also thought that I'll create a special semaphore (ExternalSemaphore -
> which is a subclass of Semaphore) that knows its own index in the external objects array. This could be really handy for finding the object in the array, but the VM is not willing to signal these semaphores in the external objects array. I thought that this is because the VM doesn't know anything about this class.
> So I tried to change Semaphore itself, but after the change the VM refused to signal any semaphores in the external objects array. Changing the class back to the original didn't help either. Without looking at the VM code, I assumed that the VM will signal anything which has the class in the 19th slot of the special objects array, but that doesn't seem to be the case.
> Is there any way to make this (add an instatnce variable to Semaphore and still get signals from the VM, or use instances of a subclass of Semaphore there) work?

The VM only checks for the exact class in slot 19, not for subclasses. Adding an inst var to Semaphore itself should be fine, but you cannot use a subclass without changing the VM. Are you sure you put the modified Semaphore class into SpecialObjectsArray after adding the inst var?

- Bert -

smime.p7s (5K) Download Attachment

Levente Uzonyi-2

Re: Questions about external objects

Thanks Bert. It turned out that my image was already broken when I tried
to add an instance variable to Semaphore.
I've finished the implementation and uploaded it to The Inbox. The
following snippet will load it into an updated Trunk image:

Installer mc
http: 'source.squeak.org';
project: 'inbox';
package: 'Kernel-ul.881';
package: 'System-ul.686';
package: 'Kernel-ul.882';
package: 'Sound-ul.39';
package: 'Network-ul.153';
package: 'Files-ul.139';
install

I decided to go with the complex solution, which gives the best
performance, and a simple API for Semaphores, but also has pretty good
performance for any other object. And it's backwards compatible in almost
all cases.

Levente

On Sun, 12 Oct 2014, Bert Freudenberg wrote:

> On 12.10.2014, at 08:04, Levente Uzonyi <[hidden email]> wrote:
>
>> Hi,
>>
>> I found that the handling of external objects is suboptimal in the image, and I came up with some ideas to make it better. I have a working solution in my image, but it can be even better.
>>
>> It seems to me that only Semaphores are stored in the external objects array (slot 39 in the special objects array), but in practice any object can be stored there. Does it make sense to support objects of any other kind? If not, then the code could be simplified.
>
> Currently the VM uses the ExternalObjectsArray only for semaphores. Other objects use different mechanisms (e.g. SurfacePlugin for external forms). I'm not sure if there is a good use case for storing non-semaphores in ExternalObjectsArray, maybe ask on the vm-dev list?
>
>> I also thought that I'll create a special semaphore (ExternalSemaphore -
>> which is a subclass of Semaphore) that knows its own index in the external objects array. This could be really handy for finding the object in the array, but the VM is not willing to signal these semaphores in the external objects array. I thought that this is because the VM doesn't know anything about this class.
>> So I tried to change Semaphore itself, but after the change the VM refused to signal any semaphores in the external objects array. Changing the class back to the original didn't help either. Without looking at the VM code, I assumed that the VM will signal anything which has the class in the 19th slot of the special objects array, but that doesn't seem to be the case.
>> Is there any way to make this (add an instatnce variable to Semaphore and still get signals from the VM, or use instances of a subclass of Semaphore there) work?
>
> The VM only checks for the exact class in slot 19, not for subclasses. Adding an inst var to Semaphore itself should be fine, but you cannot use a subclass without changing the VM. Are you sure you put the modified Semaphore class into SpecialObjectsArray after adding the inst var?
>
> - Bert -
>
>
>
>

Bert Freudenberg

Re: Questions about external objects

Your implementation looks good.

But if we're changing the API, can we get around exposing ExternalSemaphoreTable to user code? It is an implementation detail, after all.

E.g. for file opening:

semaphore := Semaphore new.
fileHandle := self primOpen: name asVmPathName forWrite: writeable semaIndex: semaphore registerExternalIndex.
fileHandle ifNil: [
semaphore unregisterExternalIndex.
semaphore := nil.
^ nil].

I'm not sure about the exact selectors, but this would make the programmer not having to deal with the implementation detail of the table at all.

- Bert -

On 24.10.2014, at 08:22, Levente Uzonyi <[hidden email]> wrote:

> Thanks Bert. It turned out that my image was already broken when I tried to add an instance variable to Semaphore.
> I've finished the implementation and uploaded it to The Inbox. The following snippet will load it into an updated Trunk image:
>
> Installer mc
> http: 'source.squeak.org';
> project: 'inbox';
> package: 'Kernel-ul.881';
> package: 'System-ul.686';
> package: 'Kernel-ul.882';
> package: 'Sound-ul.39';
> package: 'Network-ul.153';
> package: 'Files-ul.139';
> install
>
> I decided to go with the complex solution, which gives the best performance, and a simple API for Semaphores, but also has pretty good performance for any other object. And it's backwards compatible in almost all cases.
>
> Levente
>
> On Sun, 12 Oct 2014, Bert Freudenberg wrote:
>
>> On 12.10.2014, at 08:04, Levente Uzonyi <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> I found that the handling of external objects is suboptimal in the image, and I came up with some ideas to make it better. I have a working solution in my image, but it can be even better.
>>>
>>> It seems to me that only Semaphores are stored in the external objects array (slot 39 in the special objects array), but in practice any object can be stored there. Does it make sense to support objects of any other kind? If not, then the code could be simplified.
>>
>> Currently the VM uses the ExternalObjectsArray only for semaphores. Other objects use different mechanisms (e.g. SurfacePlugin for external forms). I'm not sure if there is a good use case for storing non-semaphores in ExternalObjectsArray, maybe ask on the vm-dev list?
>>
>>> I also thought that I'll create a special semaphore (ExternalSemaphore -
>>> which is a subclass of Semaphore) that knows its own index in the external objects array. This could be really handy for finding the object in the array, but the VM is not willing to signal these semaphores in the external objects array. I thought that this is because the VM doesn't know anything about this class.
>>> So I tried to change Semaphore itself, but after the change the VM refused to signal any semaphores in the external objects array. Changing the class back to the original didn't help either. Without looking at the VM code, I assumed that the VM will signal anything which has the class in the 19th slot of the special objects array, but that doesn't seem to be the case.
>>> Is there any way to make this (add an instatnce variable to Semaphore and still get signals from the VM, or use instances of a subclass of Semaphore there) work?
>>
>> The VM only checks for the exact class in slot 19, not for subclasses. Adding an inst var to Semaphore itself should be fine, but you cannot use a subclass without changing the VM. Are you sure you put the modified Semaphore class into SpecialObjectsArray after adding the inst var?
>>
>> - Bert -
>>
>>
>>
>>
>

smime.p7s (5K) Download Attachment

Bert Freudenberg

Re: Questions about external objects

Another thought: is it really necessary to cache the index in the semaphore? Wouldn't scanning the externalObjectsTable for self be efficient enough? It's pretty small, after all.

- Bert -

On 24.10.2014, at 11:44, Bert Freudenberg <[hidden email]> wrote:

> Your implementation looks good.
>
> But if we're changing the API, can we get around exposing ExternalSemaphoreTable to user code? It is an implementation detail, after all.
>
> E.g. for file opening:
>
> semaphore := Semaphore new.
> fileHandle := self primOpen: name asVmPathName forWrite: writeable semaIndex: semaphore registerExternalIndex.
> fileHandle ifNil: [
> semaphore unregisterExternalIndex.
> semaphore := nil.
> ^ nil].
>
> I'm not sure about the exact selectors, but this would make the programmer not having to deal with the implementation detail of the table at all.
>
> - Bert -
>
> On 24.10.2014, at 08:22, Levente Uzonyi <[hidden email]> wrote:
>
>> Thanks Bert. It turned out that my image was already broken when I tried to add an instance variable to Semaphore.
>> I've finished the implementation and uploaded it to The Inbox. The following snippet will load it into an updated Trunk image:
>>
>> Installer mc
>> http: 'source.squeak.org';
>> project: 'inbox';
>> package: 'Kernel-ul.881';
>> package: 'System-ul.686';
>> package: 'Kernel-ul.882';
>> package: 'Sound-ul.39';
>> package: 'Network-ul.153';
>> package: 'Files-ul.139';
>> install
>>
>> I decided to go with the complex solution, which gives the best performance, and a simple API for Semaphores, but also has pretty good performance for any other object. And it's backwards compatible in almost all cases.
>>
>> Levente
>>
>> On Sun, 12 Oct 2014, Bert Freudenberg wrote:
>>
>>> On 12.10.2014, at 08:04, Levente Uzonyi <[hidden email]> wrote:
>>>
>>>> Hi,
>>>>
>>>> I found that the handling of external objects is suboptimal in the image, and I came up with some ideas to make it better. I have a working solution in my image, but it can be even better.
>>>>
>>>> It seems to me that only Semaphores are stored in the external objects array (slot 39 in the special objects array), but in practice any object can be stored there. Does it make sense to support objects of any other kind? If not, then the code could be simplified.
>>>
>>> Currently the VM uses the ExternalObjectsArray only for semaphores. Other objects use different mechanisms (e.g. SurfacePlugin for external forms). I'm not sure if there is a good use case for storing non-semaphores in ExternalObjectsArray, maybe ask on the vm-dev list?
>>>
>>>> I also thought that I'll create a special semaphore (ExternalSemaphore -
>>>> which is a subclass of Semaphore) that knows its own index in the external objects array. This could be really handy for finding the object in the array, but the VM is not willing to signal these semaphores in the external objects array. I thought that this is because the VM doesn't know anything about this class.
>>>> So I tried to change Semaphore itself, but after the change the VM refused to signal any semaphores in the external objects array. Changing the class back to the original didn't help either. Without looking at the VM code, I assumed that the VM will signal anything which has the class in the 19th slot of the special objects array, but that doesn't seem to be the case.
>>>> Is there any way to make this (add an instatnce variable to Semaphore and still get signals from the VM, or use instances of a subclass of Semaphore there) work?
>>>
>>> The VM only checks for the exact class in slot 19, not for subclasses. Adding an inst var to Semaphore itself should be fine, but you cannot use a subclass without changing the VM. Are you sure you put the modified Semaphore class into SpecialObjectsArray after adding the inst var?
>>>
>>> - Bert -
>>>
>>>
>>>
>>>
>>
>
>
>

smime.p7s (5K) Download Attachment

Levente Uzonyi-2

Re: Questions about external objects

On Fri, 24 Oct 2014, Bert Freudenberg wrote:

> Another thought: is it really necessary to cache the index in the semaphore? Wouldn't scanning the externalObjectsTable for self be efficient enough? It's pretty small, after all.

That's exactly what the current implementation does (scans the table), and
this is what I want to avoid, because it doesn't scale. The table is small
when you start your image, but it can easily grow large - especially on a
server (3 semaphores per socket) - and it never gets shrinked. So once it
grows, it'll stay slow "forever".

And no, it's not absolutely necessary to store the index in the
semaphores. The new implementation uses an IdentityDictionary to store the
indexes of non-semaphores. The same method could work for semaphores too.
But it's convenient to store the index in the semaphore, otherwise the
method which creates and registers a new semaphore has to return two
values - the index and the semaphore - which is cumbersome.
It's also faster to ask for a new Semaphore from the
ExternalSemaphoreTable, because that way it knows that the
Semaphore is not registered.

>
> - Bert -
>
> On 24.10.2014, at 11:44, Bert Freudenberg <[hidden email]> wrote:
>
>> Your implementation looks good.
>>
>> But if we're changing the API, can we get around exposing ExternalSemaphoreTable to user code? It is an implementation detail, after all.
>>
>> E.g. for file opening:
>>
>> semaphore := Semaphore new.
>> fileHandle := self primOpen: name asVmPathName forWrite: writeable semaIndex: semaphore registerExternalIndex.
>> fileHandle ifNil: [
>> semaphore unregisterExternalIndex.
>> semaphore := nil.
>> ^ nil].
>>
>> I'm not sure about the exact selectors, but this would make the programmer not having to deal with the implementation detail of the table at all.

Yes, it's possible to extend the API by adding a few methods to
Semaphore.

Levente

>>
>> - Bert -
>>
>> On 24.10.2014, at 08:22, Levente Uzonyi <[hidden email]> wrote:
>>
>>> Thanks Bert. It turned out that my image was already broken when I tried to add an instance variable to Semaphore.
>>> I've finished the implementation and uploaded it to The Inbox. The following snippet will load it into an updated Trunk image:
>>>
>>> Installer mc
>>> http: 'source.squeak.org';
>>> project: 'inbox';
>>> package: 'Kernel-ul.881';
>>> package: 'System-ul.686';
>>> package: 'Kernel-ul.882';
>>> package: 'Sound-ul.39';
>>> package: 'Network-ul.153';
>>> package: 'Files-ul.139';
>>> install
>>>
>>> I decided to go with the complex solution, which gives the best performance, and a simple API for Semaphores, but also has pretty good performance for any other object. And it's backwards compatible in almost all cases.
>>>
>>> Levente
>>>
>>> On Sun, 12 Oct 2014, Bert Freudenberg wrote:
>>>
>>>> On 12.10.2014, at 08:04, Levente Uzonyi <[hidden email]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I found that the handling of external objects is suboptimal in the image, and I came up with some ideas to make it better. I have a working solution in my image, but it can be even better.
>>>>>
>>>>> It seems to me that only Semaphores are stored in the external objects array (slot 39 in the special objects array), but in practice any object can be stored there. Does it make sense to support objects of any other kind? If not, then the code could be simplified.
>>>>
>>>> Currently the VM uses the ExternalObjectsArray only for semaphores. Other objects use different mechanisms (e.g. SurfacePlugin for external forms). I'm not sure if there is a good use case for storing non-semaphores in ExternalObjectsArray, maybe ask on the vm-dev list?
>>>>
>>>>> I also thought that I'll create a special semaphore (ExternalSemaphore -
>>>>> which is a subclass of Semaphore) that knows its own index in the external objects array. This could be really handy for finding the object in the array, but the VM is not willing to signal these semaphores in the external objects array. I thought that this is because the VM doesn't know anything about this class.
>>>>> So I tried to change Semaphore itself, but after the change the VM refused to signal any semaphores in the external objects array. Changing the class back to the original didn't help either. Without looking at the VM code, I assumed that the VM will signal anything which has the class in the 19th slot of the special objects array, but that doesn't seem to be the case.
>>>>> Is there any way to make this (add an instatnce variable to Semaphore and still get signals from the VM, or use instances of a subclass of Semaphore there) work?
>>>>
>>>> The VM only checks for the exact class in slot 19, not for subclasses. Adding an inst var to Semaphore itself should be fine, but you cannot use a subclass without changing the VM. Are you sure you put the modified Semaphore class into SpecialObjectsArray after adding the inst var?
>>>>
>>>> - Bert -
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
>
>

David T. Lewis

Re: Questions about external objects

On Fri, Oct 24, 2014 at 09:52:11PM +0200, Levente Uzonyi wrote:

> On Fri, 24 Oct 2014, Bert Freudenberg wrote:
>
> >Another thought: is it really necessary to cache the index in the
> >semaphore? Wouldn't scanning the externalObjectsTable for self be
> >efficient enough? It's pretty small, after all.
>
> That's exactly what the current implementation does (scans the table), and
> this is what I want to avoid, because it doesn't scale. The table is small
> when you start your image, but it can easily grow large - especially on a
> server (3 semaphores per socket) - and it never gets shrinked. So once it
> grows, it'll stay slow "forever".
>
> And no, it's not absolutely necessary to store the index in the
> semaphores. The new implementation uses an IdentityDictionary to store the
> indexes of non-semaphores. The same method could work for semaphores too.
> But it's convenient to store the index in the semaphore, otherwise the
> method which creates and registers a new semaphore has to return two
> values - the index and the semaphore - which is cumbersome.
> It's also faster to ask for a new Semaphore from the
> ExternalSemaphoreTable, because that way it knows that the
> Semaphore is not registered.

I also like the implementation overall, but it seems to me that a Semaphore
should not be responsible for knowing about whether somebody used it in the
external objects array. It seems more natural for ExternalSemaphoreTable
to be responsible for keeping track of the relationship between semaphores
and external resources. Thus I don't really like the #indexInExternalObjectsArray
instance variable in Semaphore.

On the other hand, in your earlier post introducing this topic, you said:

> I also thought that I'll create a special semaphore (ExternalSemaphore -
> which is a subclass of Semaphore) that knows its own index in the external
> objects array.

So IMO, maybe maybe your original concept is better, even if it requires
another class. An ExternalSemaphore would be a Semaphore that knows about
its place in the ExternalObjectsArray, but semaphores in general do not
need to know about that.

I'm not sure which would be better, but it seems to me that either
ExternalSemaphoreTable should be responsible for maintaining the relationship
(presumably with an IdentityDictionary), or ExternalSemaphore should be
a kind of semaphore that knows something more about its place in the world.

Dave

Levente Uzonyi-2

Re: Questions about external objects

On Fri, 24 Oct 2014, David T. Lewis wrote:

> On Fri, Oct 24, 2014 at 09:52:11PM +0200, Levente Uzonyi wrote:
>> On Fri, 24 Oct 2014, Bert Freudenberg wrote:
>>
>>> Another thought: is it really necessary to cache the index in the
>>> semaphore? Wouldn't scanning the externalObjectsTable for self be
>>> efficient enough? It's pretty small, after all.
>>
>> That's exactly what the current implementation does (scans the table), and
>> this is what I want to avoid, because it doesn't scale. The table is small
>> when you start your image, but it can easily grow large - especially on a
>> server (3 semaphores per socket) - and it never gets shrinked. So once it
>> grows, it'll stay slow "forever".
>>
>> And no, it's not absolutely necessary to store the index in the
>> semaphores. The new implementation uses an IdentityDictionary to store the
>> indexes of non-semaphores. The same method could work for semaphores too.
>> But it's convenient to store the index in the semaphore, otherwise the
>> method which creates and registers a new semaphore has to return two
>> values - the index and the semaphore - which is cumbersome.
>> It's also faster to ask for a new Semaphore from the
>> ExternalSemaphoreTable, because that way it knows that the
>> Semaphore is not registered.
>
> I also like the implementation overall, but it seems to me that a Semaphore
> should not be responsible for knowing about whether somebody used it in the
> external objects array. It seems more natural for ExternalSemaphoreTable
> to be responsible for keeping track of the relationship between semaphores
> and external resources. Thus I don't really like the #indexInExternalObjectsArray
> instance variable in Semaphore.
>
> On the other hand, in your earlier post introducing this topic, you said:
>
>> I also thought that I'll create a special semaphore (ExternalSemaphore -
>> which is a subclass of Semaphore) that knows its own index in the external
>> objects array.
>
> So IMO, maybe maybe your original concept is better, even if it requires
> another class. An ExternalSemaphore would be a Semaphore that knows about
> its place in the ExternalObjectsArray, but semaphores in general do not
> need to know about that.
>
> I'm not sure which would be better, but it seems to me that either
> ExternalSemaphoreTable should be responsible for maintaining the relationship
> (presumably with an IdentityDictionary), or ExternalSemaphore should be
> a kind of semaphore that knows something more about its place in the world.

The problem with ExternalSemaphore idea is that it requires VM changes.
The VM is not willing to signal instances of non-Semaphores. AFAIK
signaling is based on what's in slot 19 of the specialObjectsArray. But
changing the class there is not possible, because other VM mechanisms also
rely on the content of that slot.

Levente

>
> Dave
>
>
>

David T. Lewis

Re: Questions about external objects

On Sat, Oct 25, 2014 at 01:07:31AM +0200, Levente Uzonyi wrote:

> On Fri, 24 Oct 2014, David T. Lewis wrote:
>
> >On Fri, Oct 24, 2014 at 09:52:11PM +0200, Levente Uzonyi wrote:
> >>On Fri, 24 Oct 2014, Bert Freudenberg wrote:
> >>
> >>>Another thought: is it really necessary to cache the index in the
> >>>semaphore? Wouldn't scanning the externalObjectsTable for self be
> >>>efficient enough? It's pretty small, after all.
> >>
> >>That's exactly what the current implementation does (scans the table), and
> >>this is what I want to avoid, because it doesn't scale. The table is small
> >>when you start your image, but it can easily grow large - especially on a
> >>server (3 semaphores per socket) - and it never gets shrinked. So once it
> >>grows, it'll stay slow "forever".
> >>
> >>And no, it's not absolutely necessary to store the index in the
> >>semaphores. The new implementation uses an IdentityDictionary to store the
> >>indexes of non-semaphores. The same method could work for semaphores too.
> >>But it's convenient to store the index in the semaphore, otherwise the
> >>method which creates and registers a new semaphore has to return two
> >>values - the index and the semaphore - which is cumbersome.
> >>It's also faster to ask for a new Semaphore from the
> >>ExternalSemaphoreTable, because that way it knows that the
> >>Semaphore is not registered.
> >
> >I also like the implementation overall, but it seems to me that a Semaphore
> >should not be responsible for knowing about whether somebody used it in the
> >external objects array. It seems more natural for ExternalSemaphoreTable
> >to be responsible for keeping track of the relationship between semaphores
> >and external resources. Thus I don't really like the
> >#indexInExternalObjectsArray
> >instance variable in Semaphore.
> >
> >On the other hand, in your earlier post introducing this topic, you said:
> >
> >> I also thought that I'll create a special semaphore (ExternalSemaphore -
> >> which is a subclass of Semaphore) that knows its own index in the
> >> external
> >> objects array.
> >
> >So IMO, maybe maybe your original concept is better, even if it requires
> >another class. An ExternalSemaphore would be a Semaphore that knows about
> >its place in the ExternalObjectsArray, but semaphores in general do not
> >need to know about that.
> >
> >I'm not sure which would be better, but it seems to me that either
> >ExternalSemaphoreTable should be responsible for maintaining the
> >relationship
> >(presumably with an IdentityDictionary), or ExternalSemaphore should be
> >a kind of semaphore that knows something more about its place in the world.
>
> The problem with ExternalSemaphore idea is that it requires VM changes.
> The VM is not willing to signal instances of non-Semaphores. AFAIK
> signaling is based on what's in slot 19 of the specialObjectsArray. But
> changing the class there is not possible, because other VM mechanisms also
> rely on the content of that slot.

Hmm... Unfortunately you are right.

InterpreterPrimitives>>primitiveSignal
"synchromously signal the semaphore. This may change the active process as a result"
| sema |
sema := self stackTop. "rcvr"
self assertClassOf: sema is: (objectMemory splObj: ClassSemaphore).
self successful ifTrue: [ self synchronousSignal: sema ].

Dave

David T. Lewis

Re: Questions about external objects

In reply to this post by Levente Uzonyi-2

On Fri, Oct 24, 2014 at 09:52:11PM +0200, Levente Uzonyi wrote:

I don't see the indexesByObjects dictionary actually being updated, except
in the case of setting a new externalObjectsArray in the ExternalSemaphoreTable.
I think I am overlooking something?

Dave

Levente Uzonyi-2

Re: Questions about external objects

On Sat, 25 Oct 2014, David T. Lewis wrote:

It's updated in #safelyRegisterExternalObject: and
#safelyUnregisterExternalObject:, but - as the class comment states - it's
only used for non-Semaphores.

Based on the feedback there's too much resistance to add an instance
variable to Semaphore, so I'm about to remove that part from the
implementation.

Levente

>
> Dave
>
>
>

David T. Lewis

Re: Questions about external objects

On Sat, Oct 25, 2014 at 10:02:22PM +0200, Levente Uzonyi wrote:

> On Sat, 25 Oct 2014, David T. Lewis wrote:
>
> >On Fri, Oct 24, 2014 at 09:52:11PM +0200, Levente Uzonyi wrote:
> >>On Fri, 24 Oct 2014, Bert Freudenberg wrote:
> >>
> >>>Another thought: is it really necessary to cache the index in the
> >>>semaphore? Wouldn't scanning the externalObjectsTable for self be
> >>>efficient enough? It's pretty small, after all.
> >>
> >>That's exactly what the current implementation does (scans the table), and
> >>this is what I want to avoid, because it doesn't scale. The table is small
> >>when you start your image, but it can easily grow large - especially on a
> >>server (3 semaphores per socket) - and it never gets shrinked. So once it
> >>grows, it'll stay slow "forever".
> >>
> >>And no, it's not absolutely necessary to store the index in the
> >>semaphores. The new implementation uses an IdentityDictionary to store the
> >>indexes of non-semaphores. The same method could work for semaphores too.
> >>But it's convenient to store the index in the semaphore, otherwise the
> >>method which creates and registers a new semaphore has to return two
> >>values - the index and the semaphore - which is cumbersome.
> >>It's also faster to ask for a new Semaphore from the
> >>ExternalSemaphoreTable, because that way it knows that the
> >>Semaphore is not registered.
> >>
> >
> >I don't see the indexesByObjects dictionary actually being updated, except
> >in the case of setting a new externalObjectsArray in the
> >ExternalSemaphoreTable.
> >I think I am overlooking something?
>
> It's updated in #safelyRegisterExternalObject: and
> #safelyUnregisterExternalObject:, but - as the class comment states - it's
> only used for non-Semaphores.
>
> Based on the feedback there's too much resistance to add an instance
> variable to Semaphore, so I'm about to remove that part from the
> implementation.

I don't think the instance variable is horrible, but I do think it would
feel cleaner if you can do without it, so +1 if you are able to do that.

Thanks
Dave

David T. Lewis

Re: Questions about external objects

In reply to this post by Levente Uzonyi-2

On Sat, Oct 25, 2014 at 10:02:22PM +0200, Levente Uzonyi wrote:

> On Sat, 25 Oct 2014, David T. Lewis wrote:
> >
> >I don't see the indexesByObjects dictionary actually being updated, except
> >in the case of setting a new externalObjectsArray in the
> >ExternalSemaphoreTable.
> >I think I am overlooking something?
>
> It's updated in #safelyRegisterExternalObject: and
> #safelyUnregisterExternalObject:, but - as the class comment states - it's
> only used for non-Semaphores.

Sorry, I should have looked more carefully. This works exactly as expected
and as documented in the class comment:

ExternalSemaphoreTable registerExternalObject: Object new.

Dave

Levente Uzonyi-2

Re: Questions about external objects

In reply to this post by David T. Lewis

I've uploaded the new versions of the packages to The Inbox, and moved the
old ones to The Treated Inbox.
The main difference is that the Semaphore class is not modified, and its
instances are handled like any other object. There's also a new #cleanUp:
method which will allow freeing up some memory. And the caches are
shrinked whenever the external objects are wiped.
Since Semaphores don't know their indexes anymore, the new part of the API
has also changed.
The following snippet will load the new versions to an up-to-date Trunk
image:

Installer squeakInbox
package: 'System-ul.687';
package: 'Sound-ul.40';
package: 'Kernel-ul.883';
package: 'Network-ul.154';
package: 'Files-ul.140';
install

Levente

On Sat, 25 Oct 2014, David T. Lewis wrote:

> On Sat, Oct 25, 2014 at 10:02:22PM +0200, Levente Uzonyi wrote:
>> On Sat, 25 Oct 2014, David T. Lewis wrote:
>>
>>> On Fri, Oct 24, 2014 at 09:52:11PM +0200, Levente Uzonyi wrote:
>>>> On Fri, 24 Oct 2014, Bert Freudenberg wrote:
>>>>
>>>>> Another thought: is it really necessary to cache the index in the
>>>>> semaphore? Wouldn't scanning the externalObjectsTable for self be
>>>>> efficient enough? It's pretty small, after all.
>>>>
>>>> That's exactly what the current implementation does (scans the table), and
>>>> this is what I want to avoid, because it doesn't scale. The table is small
>>>> when you start your image, but it can easily grow large - especially on a
>>>> server (3 semaphores per socket) - and it never gets shrinked. So once it
>>>> grows, it'll stay slow "forever".
>>>>
>>>> And no, it's not absolutely necessary to store the index in the
>>>> semaphores. The new implementation uses an IdentityDictionary to store the
>>>> indexes of non-semaphores. The same method could work for semaphores too.
>>>> But it's convenient to store the index in the semaphore, otherwise the
>>>> method which creates and registers a new semaphore has to return two
>>>> values - the index and the semaphore - which is cumbersome.
>>>> It's also faster to ask for a new Semaphore from the
>>>> ExternalSemaphoreTable, because that way it knows that the
>>>> Semaphore is not registered.
>>>>
>>>
>>> I don't see the indexesByObjects dictionary actually being updated, except
>>> in the case of setting a new externalObjectsArray in the
>>> ExternalSemaphoreTable.
>>> I think I am overlooking something?
>>
>> It's updated in #safelyRegisterExternalObject: and
>> #safelyUnregisterExternalObject:, but - as the class comment states - it's
>> only used for non-Semaphores.
>>
>> Based on the feedback there's too much resistance to add an instance
>> variable to Semaphore, so I'm about to remove that part from the
>> implementation.
>
> I don't think the instance variable is horrible, but I do think it would
> feel cleaner if you can do without it, so +1 if you are able to do that.
>
> Thanks
> Dave
>
>
>

David T. Lewis

Re: Questions about external objects

On Sun, Oct 26, 2014 at 02:06:20AM +0200, Levente Uzonyi wrote:

> I've uploaded the new versions of the packages to The Inbox, and moved the
> old ones to The Treated Inbox.
> The main difference is that the Semaphore class is not modified, and its
> instances are handled like any other object. There's also a new #cleanUp:
> method which will allow freeing up some memory. And the caches are
> shrinked whenever the external objects are wiped.
> Since Semaphores don't know their indexes anymore, the new part of the API
> has also changed.
> The following snippet will load the new versions to an up-to-date Trunk
> image:
>
> Installer squeakInbox
> package: 'System-ul.687';
> package: 'Sound-ul.40';
> package: 'Kernel-ul.883';
> package: 'Network-ul.154';
> package: 'Files-ul.140';
> install
>
>
> Levente
>

I like this a lot. Not only does it provide the ExternalSemaphoreTable
improvements, but I also find that when I restart my image with OSProcess
loaded, the indexesByObjects dictionary clearly shows the semaphore that
was registered for the child process watcher at image startup. Thus an
explorer on the ExternalSemaphoreTable will display semaphores associated
with processes that wait on them, which also identifies the process waiting
on that semaphore. You can see this right in the object explorer.

Very nice!

I note that a Semaphore can still know its external objects index by
querying the ExternalSemaphoreTable:

Semaphore>>externalIndex
^ExternalSemaphoreTable externalIndexFor: self

ExternalSemaphoreTable class>>externalIndexFor: anObject
^self current indexesByObjects at: anObject ifAbsent: [nil]

ExternalSemaphoreTable>>indexesByObjects
^indexesByObjects

Dave

Levente Uzonyi-2

Re: Questions about external objects

On Sat, 25 Oct 2014, David T. Lewis wrote:

> On Sun, Oct 26, 2014 at 02:06:20AM +0200, Levente Uzonyi wrote:
>> I've uploaded the new versions of the packages to The Inbox, and moved the
>> old ones to The Treated Inbox.
>> The main difference is that the Semaphore class is not modified, and its
>> instances are handled like any other object. There's also a new #cleanUp:
>> method which will allow freeing up some memory. And the caches are
>> shrinked whenever the external objects are wiped.
>> Since Semaphores don't know their indexes anymore, the new part of the API
>> has also changed.
>> The following snippet will load the new versions to an up-to-date Trunk
>> image:
>>
>> Installer squeakInbox
>> package: 'System-ul.687';
>> package: 'Sound-ul.40';
>> package: 'Kernel-ul.883';
>> package: 'Network-ul.154';
>> package: 'Files-ul.140';
>> install
>>
>>
>> Levente
>>
>
> I like this a lot. Not only does it provide the ExternalSemaphoreTable
> improvements, but I also find that when I restart my image with OSProcess
> loaded, the indexesByObjects dictionary clearly shows the semaphore that
> was registered for the child process watcher at image startup. Thus an
> explorer on the ExternalSemaphoreTable will display semaphores associated
> with processes that wait on them, which also identifies the process waiting
> on that semaphore. You can see this right in the object explorer.
>
> Very nice!
>
> I note that a Semaphore can still know its external objects index by
> querying the ExternalSemaphoreTable:
>
> Semaphore>>externalIndex
> ^ExternalSemaphoreTable externalIndexFor: self
>
> ExternalSemaphoreTable class>>externalIndexFor: anObject
> ^self current indexesByObjects at: anObject ifAbsent: [nil]
>
> ExternalSemaphoreTable>>indexesByObjects
> ^indexesByObjects

It's doable, but I don't think it's useful. The index is only meaningful
for the plugins. Also, most of the API could be added to Semaphore too as
Bert suggested.
Btw, accessing #indexesByObjects is no thread-safe this way.

Levente

>
> Dave
>
>
>

David T. Lewis

Re: Questions about external objects

In reply to this post by Levente Uzonyi-2

On Sun, Oct 26, 2014 at 02:06:20AM +0200, Levente Uzonyi wrote:

This looks really good to me. I vote for moving it to trunk.

Dave

Bert Freudenberg

Re: Questions about external objects

On 26.10.2014, at 10:32, David T. Lewis <[hidden email]> wrote:

Yes, the implementation itself looks good.

It's just a bit unfortunate that the user code gets even less generic than it was before.

- Bert -

smime.p7s (5K) Download Attachment

Levente Uzonyi-2

Re: Questions about external objects

On Sun, 26 Oct 2014, Bert Freudenberg wrote:

>
> On 26.10.2014, at 10:32, David T. Lewis <[hidden email]> wrote:
>
>> On Sun, Oct 26, 2014 at 02:06:20AM +0200, Levente Uzonyi wrote:
>>> I've uploaded the new versions of the packages to The Inbox, and moved the
>>> old ones to The Treated Inbox.
>>> The main difference is that the Semaphore class is not modified, and its
>>> instances are handled like any other object. There's also a new #cleanUp:
>>> method which will allow freeing up some memory. And the caches are
>>> shrinked whenever the external objects are wiped.
>>> Since Semaphores don't know their indexes anymore, the new part of the API
>>> has also changed.
>>> The following snippet will load the new versions to an up-to-date Trunk
>>> image:
>>>
>>> Installer squeakInbox
>>> package: 'System-ul.687';
>>> package: 'Sound-ul.40';
>>> package: 'Kernel-ul.883';
>>> package: 'Network-ul.154';
>>> package: 'Files-ul.140';
>>> install
>>>
>>
>> This looks really good to me. I vote for moving it to trunk.
>>
>> Dave
>
> Yes, the implementation itself looks good.
>
> It's just a bit unfortunate that the user code gets even less generic than it was before.

What do you mean by "less generic"? Is it that the code says
ExternalSemaphoreTable instead of Smalltalk?
The code was extracted from Smalltalk more than 14 years ago, because

"It seemed cleaner to deligate the reponsibility here versus adding more code and another class variable to SystemDictionary"

I could have left all (indirect) users of the ExternalSemaphoreTable
unchanged, but I think we really should make SmalltalkImage less
monolithic and remove those "proxy" methods from it in the near future.
The new methods give additional performance boost (because we know that
the Semaphores are new, so we don't have to check if they are already
included, and because we don't have to wait for the lock multiple times,
when multiple Semaphores are registered or unregistered) especially for
Sockets, which was the main reason I spent so much time on this.

Levente

>
> - Bert -
>
>

David T. Lewis

Re: Questions about external objects

In reply to this post by Bert Freudenberg

On Sun, Oct 26, 2014 at 12:36:13PM -0400, Bert Freudenberg wrote:

>
> On 26.10.2014, at 10:32, David T. Lewis <[hidden email]> wrote:
>
> > On Sun, Oct 26, 2014 at 02:06:20AM +0200, Levente Uzonyi wrote:
> >> I've uploaded the new versions of the packages to The Inbox, and moved the
> >> old ones to The Treated Inbox.
> >> The main difference is that the Semaphore class is not modified, and its
> >> instances are handled like any other object. There's also a new #cleanUp:
> >> method which will allow freeing up some memory. And the caches are
> >> shrinked whenever the external objects are wiped.
> >> Since Semaphores don't know their indexes anymore, the new part of the API
> >> has also changed.
> >> The following snippet will load the new versions to an up-to-date Trunk
> >> image:
> >>
> >> Installer squeakInbox
> >> package: 'System-ul.687';
> >> package: 'Sound-ul.40';
> >> package: 'Kernel-ul.883';
> >> package: 'Network-ul.154';
> >> package: 'Files-ul.140';
> >> install
> >>
> >
> > This looks really good to me. I vote for moving it to trunk.
> >
> > Dave
>
> Yes, the implementation itself looks good.
>
> It's just a bit unfortunate that the user code gets even less generic than it was before.
>

Bert,

Are you referring to user code that calls e.g. newExternalSemaphoreDo: and
receives a semaphore and its index in the external table? This seems quite
reasonable to me, and it reads nicely in the Network, File, and Sound code
that sends it. Meanwhile, existing code such as OSProcess continues to work
without modification.

Dave