[Pharo-project] About announcements
tudor at tudorgirba.com
Mon Jan 2 09:24:04 EST 2012
Thanks Lukas for the detailed points. These are great because they are to the point and they lead to hands-on solutions.
And, thanks Igor for picking it up. If Henrik would join the discussion we would have all parties involved and action can just start.
And it was so easy.
On 2 Jan 2012, at 14:45, 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:
>>> 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
> 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.
> 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
> 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 Renggli
> Best regards,
> Igor Stasenko.
"Some battles are better lost than fought."
More information about the Pharo-dev