Smalltalk › Pharo › Pharo Smalltalk Developers

About announcements

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

16 messages Options

Igor Stasenko

About announcements

On 2 January 2012 10:42, Lukas Renggli <[hidden email]> 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.

Stéphane Ducasse

Re: About announcements

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 <[hidden email]> 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.
>

Tudor Girba-2

Re: About announcements

In reply to this post by Igor Stasenko

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.

Cheers,
Doru

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

--
www.tudorgirba.com

"Some battles are better lost than fought."

Stéphane Ducasse

Re: About announcements

> 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.

indeed constructivist feedback (even negative).
We were working on event and it also seems that Monitor in announcement was causing some problems.
Now I would really like to know if the Monitor implementation is good.

Stef

>
> Cheers,
> Doru
>
>
>
> On 2 Jan 2012, at 14:45, Igor Stasenko wrote:
>
>> On 2 January 2012 10:42, Lukas Renggli <[hidden email]> 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.
>>
>
> --
> www.tudorgirba.com
>
> "Some battles are better lost than fought."
>
>
>
>

Henrik Sperre Johansen

Re: About announcements

In reply to this post by Igor Stasenko

On 02.01.2012 14:45, Igor Stasenko wrote:

Yup, leftover from live migration.

>> 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.

Yup, leftover from live migration/when whole delivery was in critical
section (ie before copying), a Semaphore should be ok. Can't remember
why we picked Monitor over Mutex in the first place :(
>
>> 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.
Alternative is the way the old code worked: Copy and replace collection
whenever you add/remove subscriptions.
Then there need be no critical section in deliver: at all.
Of course, it's a performance tradeoff between fast
subscription/unsubscription and delivery, one is not necessarily better
than the other.

Question is if it's worth providing/switching between different
SubscriptionRegistries based on actual use pattern?
Or, is a default generally better, and if so, which of the approaches
should be favored?
Lukas seem to thing delivery, and I'm inclined to agree with him,
I think we were a bit blinded by benching lots of
subscriptions/unsubscriptions in relations with the weak
implementations, and lost sight of the most common use pattern.

>
>> 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
With subs parameter a sequenceable collection, it could be made nearly
non-existant by iterating manually rather than through do:, moving
ifCurtailed to the outer scope. (resuming from index rather than a copy
after failed delivery).
Code would be uglier though.

>
>
>
>> 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.

6 and 8. both arrived as responses to a desire to make delivery more robust.
I personally argued for using 6, but not 8, for a default announcer as a
reasonable(?) compromise.
Would not mind seeing this pluggable, and in either case moved to
SubscriptionRegistry to sit alongside the curtailing, so there is only
one class to modify for testing different delivery/error
handling/robustness strategies.
>
>> 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.
Personally, it was to make it slightly more compatible with VW
announcements (which also pass the subscription as an optional 3rd param).
One use is when weak announcements don't give strong enough guarantees
on termination, and your block does not have access to the announcer.
(usually when an object is passively registered for listening)
A >> handlerBlock
[:announcement :announcer | self isValid
ifTrue: [self writeSomeStateUsedElseWhereBasedOn: announcement ]
ifFalse: [announcer unsubscribe: self]]

A listenTo: announcer
announcer when: IWantToListen do: self handlerBlock

Yes, you can also do the same by keeping an extra inst var with all
announcers A are sent as arguments, personally I find that less
desirable to maintain.

What I didn't notice at the time though, was that cull: makes
MessageSend subscription delivery quite slow, due to String numArgs
implementation...
So that specific case needs optimization, otherwise the overhead of
supporting this is negligible.

Cheers,
Henry

Henrik Sperre Johansen

Re: About announcements

In reply to this post by Igor Stasenko

On 02.01.2012 15:50, Henrik Sperre Johansen wrote:

> On 02.01.2012 14:45, Igor Stasenko wrote:
>> On 2 January 2012 10:42, Lukas Renggli<[hidden email]> wrote:
>>
>>> 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
> With subs parameter a sequenceable collection, it could be made nearly
> non-existant by iterating manually rather than through do:, moving
> ifCurtailed to the outer scope. (resuming from index rather than a
> copy after failed delivery).
> Code would be uglier though.

By "nearly non-existant", I meant you pay the overhead once per delivery
rather than once per subscriber.
It's still an overhead when you have a large number of deliveries to a
few subscribers.
>> 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.
Forgot to comment this, I partly agree with both really.
Of course it is not pointless to group them, algorithmically speaking.
Is the scale at which it matters a usual occurance? Don't think so.

Do you fix your announcement lookup by doing so? Yes.
Is that necessarily a bad thing? I'm not sure.

IIRC, it was one of the discussions Igor "won" (plus, it was simpler to
mess about with) :)

Cheers,
Henry

Lukas Renggli

Re: About announcements

In reply to this post by Stéphane Ducasse

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.

>>> 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).

>>> 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.

>>> 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.

Lukas

--
Lukas Renggli
www.lukas-renggli.ch

Stéphane Ducasse

Re: About announcements

Can one of you turn the following list into bug entries?
So that we can make progress by killing them one by one?

Stef

>>>> 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.

But apparently Mutex is broken :(

>
>>>> 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).
>
>>>> 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.
>
>>>> 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.

Lukas our state of mind is that we want the infrastructure gets better so we like to change
for a better one if this better one is better.
If everybody has its own version then we do not improve the infrastructure.

Stef

Lukas Renggli

Re: About announcements

>> 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.
>
> Lukas our state of mind is that we want the infrastructure gets better so we like to change
> for a better one if this better one is better.

Seems that the understanding of what is better/worse and what is
progress/regress is rather different among people.

> If everybody has its own version then we do not improve the infrastructure.

Performance issues aside, the current implementation forces me to use
a different implementation because Pharo's announcements have a
different implementation of selecting the handler (point 7). However,
the really weird thing is that Pharo's announcements are now suddenly
inconsistent with the Pharo exceptions and how they select the
handler. Do you think this is better or worse?

Lukas

--
Lukas Renggli
www.lukas-renggli.ch

Igor Stasenko

Re: About announcements

In reply to this post by Lukas Renggli

On 2 January 2012 16:12, Lukas Renggli <[hidden email]> 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.

Igor Stasenko

Re: About announcements

In reply to this post by Lukas Renggli

On 2 January 2012 21:04, Lukas Renggli <[hidden email]> wrote:

>>> 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.
>>
>> Lukas our state of mind is that we want the infrastructure gets better so we like to change
>> for a better one if this better one is better.
>
> Seems that the understanding of what is better/worse and what is
> progress/regress is rather different among people.
>
>> If everybody has its own version then we do not improve the infrastructure.
>
> Performance issues aside, the current implementation forces me to use
> a different implementation because Pharo's announcements have a
> different implementation of selecting the handler (point 7). However,
> the really weird thing is that Pharo's announcements are now suddenly
> inconsistent with the Pharo exceptions and how they select the
> handler. Do you think this is better or worse?
>

you lost me here. why announcements have to be consistent with
exceptions? something which is completely
orthogonal to them?
it is like saying that apples are inconsistent with oranges.

aside of that, i added the changeset, so you can now choose whether
your announcement subscription is handling
announcement or not based on announcement object itself, not its class.
see http://code.google.com/p/pharo/issues/detail?id=5145

--
Best regards,
Igor Stasenko.

Lukas Renggli

Re: About announcements

In reply to this post by Igor Stasenko

>> 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

Yes, #handles: should take an instance not a class. And #handles:
should be implemented as

Announcement class>>#handles: anAnnouncement
^ anAnnouncement isKindOf: self

I guess this is partially my fault, because I called the argument
anAnnouncementClass even if I passed in an instance.

Lukas

--
Lukas Renggli
www.lukas-renggli.ch

Lukas Renggli

Re: About announcements

In reply to this post by Igor Stasenko

>>> If everybody has its own version then we do not improve the infrastructure.
>>
>> Performance issues aside, the current implementation forces me to use
>> a different implementation because Pharo's announcements have a
>> different implementation of selecting the handler (point 7). However,
>> the really weird thing is that Pharo's announcements are now suddenly
>> inconsistent with the Pharo exceptions and how they select the
>> handler. Do you think this is better or worse?
>>
> you lost me here. why announcements have to be consistent with
> exceptions? something which is completely
> orthogonal to them?

Exceptions and Announcements used to implement a very similar
protocol: see their implementation of X class>>#handles:, X>>on:do:,
XSet>>#handles:, XSet>>#, etc.

Consistency makes frameworks easy to understand and use.

> it is like saying that apples are inconsistent with oranges.

- Apples and oranges are inconsistent the way they are eaten (oranges
have to be piled first and come in slices).

- Apples and pears are consistent the way they are eaten (even if they
taste very different).

If that is too abstract replace

Apples => Announcements
Oranges => change:/update:
Pears => Exceptions

Lukas

>
> aside of that, i added the changeset, so you can now choose whether
> your announcement subscription is handling
> announcement or not based on announcement object itself, not its class.
> see http://code.google.com/p/pharo/issues/detail?id=5145
>
> --
> Best regards,
> Igor Stasenko.
>

--
Lukas Renggli
www.lukas-renggli.ch

Igor Stasenko

Re: About announcements

On 3 January 2012 14:20, Lukas Renggli <[hidden email]> wrote:

>>>> If everybody has its own version then we do not improve the infrastructure.
>>>
>>> Performance issues aside, the current implementation forces me to use
>>> a different implementation because Pharo's announcements have a
>>> different implementation of selecting the handler (point 7). However,
>>> the really weird thing is that Pharo's announcements are now suddenly
>>> inconsistent with the Pharo exceptions and how they select the
>>> handler. Do you think this is better or worse?
>>>
>> you lost me here. why announcements have to be consistent with
>> exceptions? something which is completely
>> orthogonal to them?
>
> Exceptions and Announcements used to implement a very similar
> protocol: see their implementation of X class>>#handles:, X>>on:do:,
> XSet>>#handles:, XSet>>#, etc.
>

as well as it could serve as a source of confusion, because you using
same protocol for different purposes
in two different frameworks.
A medal always has two sides :)

> Consistency makes frameworks easy to understand and use.
>
>> it is like saying that apples are inconsistent with oranges.
>
> - Apples and oranges are inconsistent the way they are eaten (oranges
> have to be piled first and come in slices).
>
> - Apples and pears are consistent the way they are eaten (even if they
> taste very different).
>
> If that is too abstract replace
>
> Apples => Announcements
> Oranges => change:/update:
> Pears => Exceptions
>

your analogy having a small glitch: all those objects sharing common
property - they can be eaten.

now in smalltalk you can do anything, any object can be 'eaten'. but
it doesn't means that they should,
otherwise we're just put all behavior in Object class, and use
subclasses as a dumb state holders
(color, taste, hasToBePiled etc)

Exceptions are conceptually different from Announcements in a way they
should be used and handled,
and i don't see a great value of having synergy between them.
If we keep driving analogies, i think a more precise would be to tell
that exceptions are to be 'eaten' and
announcements are to be 'heard'. And in that way there is no consistency at all.

Actually a logic with using inheritance chain to determine whether
announcement should be delivered to
subscription or not is IMO a major flaw, borrowed from Exceptions.
Because testing a class membership is an expensive operation and if
you are so concerned about performance,
this is a first thing which you would be likely remove.

> Lukas
>
>
>
>>
>> aside of that, i added the changeset, so you can now choose whether
>> your announcement subscription is handling
>> announcement or not based on announcement object itself, not its class.
>> see http://code.google.com/p/pharo/issues/detail?id=5145
>>
>> --
>> Best regards,
>> Igor Stasenko.
>>
>
>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>

--
Best regards,
Igor Stasenko.

Sean P. DeNigris

Re: About announcements

Administrator

Igor Stasenko wrote

as well as it could serve as a source of confusion, because you using
same protocol for different purposes
in two different frameworks.
A medal always has two sides :)

> Consistency makes frameworks easy to understand and use.

I think having the two frameworks have the same API would be much easier than if they were almost the same. And I don't think that similarity would cause confusion - quite the opposite. When working with a different framework, I've already shifted my context to the abstractions of that framework. However, my fingers naturally want to type/select-from-autocomplete/etc. the exact same things they've done before, and I don't have to look at a browser to see "how was that selector slightly different from the other one?"

Cheers,
Sean

Igor Stasenko

Re: About announcements

On 3 January 2012 16:24, Sean P. DeNigris <[hidden email]> wrote:

>
> Igor Stasenko wrote
>>
>> as well as it could serve as a source of confusion, because you using
>> same protocol for different purposes
>> in two different frameworks.
>> A medal always has two sides :)
>>
>>> Consistency makes frameworks easy to understand and use.
>>
>
> I think having the two frameworks have the same API would be much easier
> than if they were almost the same. And I don't think that similarity would
> cause confusion - quite the opposite. When working with a different
> framework, I've already shifted my context to the abstractions of that
> framework. However, my fingers naturally want to
> type/select-from-autocomplete/etc. the exact same things they've done
> before, and I don't have to look at a browser to see "how was that selector
> slightly different from the other one?"
>

exactly.
just few days ago, we were hacking new events model with Stef.

and i wrongly implemented #handleEvent:
which is sent by InputEventFetcher
and actually Sensor should just put an event into own queue,
because this message were send in fetcher process.
while event handling happens inside ui process.

with announcements i had to rename #handles: to #handlesAnnouncement:
first because if i would not do so, it would be not possible to migrate code
(because handles: expect announcement class as argument, and if i
change the sender,
it will break it)
or if i change all implementors of #handles: i will also break it
because announcers are still sending
this message with wrong argument.
well, maybe it is not a big deal. and we could just use #handles:
but imo #handlesAnnouncement: looks more explicit
(now you have less chances to be confused with the role or receiver -
an announcement filter and role of argument - an announced object)

> --
> View this message in context: http://forum.world.st/About-announcements-tp4253255p4257251.html
> Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
>

--
Best regards,
Igor Stasenko.