[Pharo-project] About announcements

Stéphane Ducasse stephane.ducasse at inria.fr
Mon Jan 2 14:50:00 CET 2012


Ok I suggest that we wait for henrik feedback and turn these points in bug entry issue.


On Jan 2, 2012, at 2:45 PM, Igor Stasenko wrote:

> On 2 January 2012 10:42, Lukas Renggli <renggli at gmail.com> wrote:
>>>>> To you, the current Pharo image is the Pharo Core and you are unhappy that it is too big (for example because RB is there).
>>>> 
>>>> The size is the least problem.
>>>> 
>>>> More annoying is that the code quickly gets out of sync and
>>>> non-changes are added to the history. Both of these problems are
>>>> already visible today.
>>>> 
>>>> Additionally, over time people will change/add/remove features that
>>>> get integrated without proper review. I just had a look at the
>>>> announcement framework in Pharo 1.4 today, it is unbelievable how a
>>>> tiny framework could degrade to a bloated, untested and dead slow pile
>>>> of mud in just a few years :-(
>>> 
>>> I think your are unfair here. The new features might be untested (current coverage is at 56%), but the changes were meant to provide working weak announcements. And they do work.
>> 
>> Nice argument for your students reading this thread:
>> http://memegenerator.net/instance/12779750.
>> 
>>> But, what do you mean by slow? How did you benchmark it?
>> 
>> No, but if you look at the code you will see many extra steps:
>> 
>> 1. It tests if a registry is there: Why would that be necessary in an
>> object-oriented implementation?
>> 
> this should be cleaned up.
> i think it is a leftover from migration code from old Announcer class (pre 1.3)
> to a new one.
> We did things incrementally, and had code to migrate all instances
> from old format to a new one,
> without losing subscription and without stopping working.
> 
>> 2. It tests if the registry is empty: Why would that be necessary in
>> an object-oriented implementation?
>> 
> not necessary. looks like a optimization.
> 
>> 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.
> 
>> 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.
> 
>> 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).
> 
> Now it is still have to iterate over inheritance chain in order to
> determine if given subscription should receive it or not.
> Of course you can put all subscriptions to Announcement into one
> group, so you don't need to check it for every subscription:
> 
> dict at: Announcement put: group.
> ....
> so you can do:
> 
> group do: [:subscription | subscripton deliver: announcement ].
> 
> but that imposing that you have a fixed model for announcement relationship,
> while with #handles: i can simply make it so, that my announcement
> class are not inherits from its superclass
> and so, even if you subscribe to Announcement, you won't receive an
> announcements of my kind, because
> it simply doesn't walks an inheritance chain in its #handles: method.
> 
> also, in the end i don't think it matters. Grouping is more optimal
> (potentially), but you won't see any difference unless you
> have hundreds of subscriptions, so i think it is not a big deal.
> Usually there are few subscriptions held by announcer. And so there is
> simply no need to bother and grouping them.
> I cannot imagine an announcer holding hundreds or thousands of subscriptions.
> 
> Announcer allSubInstances collect: [:e | e numberOfSubscriptions ]
> an OrderedCollection(0 0 1 0)
> 
>> 6. It wraps each announcement delivery into a curtailed block. It does
>> that even for those that are never actually announced.
>> 
> yeah, this can be optimized
> 
>> 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?
> 
> you can use any object as  a subscription selector, just make sure it
> understands #handles: message.
> i.e.
> 
> announcer on: myObject do: [ ... ]
> where myObject can be any object, not just Announcement (sub)class.
> 
> 
>> 8. It then wraps the actual notification into an exception handler.
>> Again, this is rarely useful and should probably rather be something
>> pluggable.
>> 
> perhaps we can add even more 'bloat' to have 'safe' and 'unsafe' announcers :)
> But for things like SystemAnnouncer, this is not a question that
> exception handler must be present
> and delivery must be guaranteed.
> 
>> 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.
> 
>> So all these 9 steps are not really necessary (or even wanted) in most
>> case. I doubt that any of them makes the code faster. I am glad that
>> the code works for you even if all of this new functionality is
>> untested. And good luck with the race condition :-)
>> 
>>> There is a point in here. But, as I said, I thought that the point is to produce the core by having jenkins strip away unwanted material. Of course, the other way would be to start from the core as a seed and have jenkins produce the current image.
>> 
>> Do you really believe that stripping away unwanted material works? No
>> other open source project in the whole world does it like that,
>> everybody builds distributions on top of a smaller core: Linux, GCC,
>> Gnome, FireFox, ... Even in the supermarket, you typically don't get a
>> vegetable hamper if you just want a some potatoes.
>> 
>> Lukas
>> 
>> --
>> Lukas Renggli
>> www.lukas-renggli.ch
>> 
> 
> 
> 
> -- 
> Best regards,
> Igor Stasenko.
> 




More information about the Pharo-project mailing list