On 2 January 2012 16:12, Lukas Renggli <renggli@gmail.com> wrote:
Ok, first of all I did not know who wrote the code. I was just fixing some of my code that suddenly broke and was astonished what the announcer all does when stepping through the announcement procedure with the debugger ...
3. It enters a critical section of a monitor: This is rarely useful and the slowest kind of concurrency control available in Pharo (a Mutex would be enough for what it is used for, and instantiate multiple semaphores and queues), and btw the lazy initialization of the monitor is the prime example of a race condition.
agreed here. I would even leave a semaphore. I think it is overlooked by Henrik. It also don't needs a lazy initialization in #protected: , since in #initialize it can just create a ready for use fresh synchronization object.
A semaphore might block, because an announcement could register another announcement. So probably the Mutex would be the right thing to use.
no, the trick there is that we leaving a critical section before returning (or calling) client code, so there is no chance that it may recurse (attempt enter critical section twice) due to some weird client code.
4. It creates a copy of the collection of all the subscriptions: This is rarely useful and wouldn't be necessary if an immutable data structure was used.
propose better solution how to deal with situation when during handling an announcement, your handler unsubscribing from announcer.
Array. Slower for registration, but faster at announcing (the common operation).
maybe.
5. It iterates over all announcements, it doesn't even try to group announcements of the same kind.
it is pointless to group them, and makes no real difference. Because announcer doesn't knows what kinds of announcements will be announcement, and it knows only about subscriptions. Suppose i have a subscription to Announcement. And i announcing AnnouncementA (a subclass of it).
Fair enough, probably it doesn't matter.
7. It then tests each announcement if it matches, again it doesn't even try to group the announcements of the same kind. Aside, the match test was changed so that it doesn't allow instance-based announcements anymore, breaking some of my code.
what is instance-based announcements?
If you have a client with 1000 different possibly dynamically changing properties and you want a way to register for announcements of a single property. The current implementation forces you to create a class per property, previously you could fake a "kind of announcements" with an instance.
you can use any object as  a subscription selector, just make sure it understands #handles: message. i.e.
No, because the Announcement>>#handles: takes a class, not an instance.
you mean here: deliver: anAnnouncement " deliver an announcement to receiver. In case of failure, it will be handled in separate process" ^ (self handles: anAnnouncement class ) ifTrue: [ [action cull: anAnnouncement cull: announcer] on: UnhandledError fork: [:ex | ex pass ]] we can simply change the 'anAnnouncement class' to just 'anAnnouncement' so the receiver of #handles: will see an announcement itself not its class. It is probably right thing to do, because then you can do: announcer on: foo do: [... ]. announcer announce: 10. Then foo will receive: foo handles: 10. and in your own #handles: implementation you can do something like: handles: anAnnouncement ^ anAnnouncement isInteger
9. It then culls the announcement and announcer into the block. Not sure why the announcer is useful, after all the block was registered with the announcer at some point.
cull there is for passing optional arguments.
I didn't complain about #cull:, but about the useless announcer passed in.
Now for me you don't need to change the semantics of the Pharo Announcements again. Now for my code I will just use my own implementation that has exactly the behavior and performance characteristics I need.
-- Best regards, Igor Stasenko.