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