Le 01/07/2015 21:29, Sven Van Caekenberghe a écrit :
On 01 Jul 2015, at 21:23, Thierry Goubier <[email protected]> wrote:
Le 01/07/2015 21:11, Sven Van Caekenberghe a écrit :
On 01 Jul 2015, at 21:07, Thierry Goubier
<[email protected]> wrote:
Le 01/07/2015 20:49, Sven Van Caekenberghe a écrit :
Reading the code from SubscriptionRegistry>>#deliver: is seems to
me that *on each announcement* a new array is created with the
subscriptions that want to handle the announcement. That strikes
me as pretty inefficient.
But, when tracing code which is a heavy user of announcements such
as MC loads, it just does not register as a loss compared to the
time lost processing those announcements.
Do you have time profiles where #deliver: register as a significant
contributor? I don't.
No, I don't either. I said 'when reading the code'. Still, it seems
odd.
A few other things are odd in this code.
I use Announcements for logging, then speed of handling is (could be)
an issue (as is multi-threaded access).
Agreed.
What I would do with that code is:
* get rid of #deliver:to:startingAt: because it is only called starting at 1.
* inline #subscriptionsHandling: into #deliver:
* inline #deliver:to: as well into #deliver: (as a combined loop)
* Check that I get the expected speed benefit (I'm probably down at half as
many lines as the previous structure, so I win on clarity/compact)
Totally agree.
It seems that the code may have been structured that way to be able to memoize
(ok, to cache) the list of interested subscribers, and it turned out it had no
sufficient impact on performance to justify managing the associated caching
logic.
Maybe, there is also the comment
"using a copy, so subscribers can unsubscribe from announcer "
Oh, unsubscribe in the code processing an announcement?
but the price to pay for this seems to high to me.
And I'm not entirely sure it protects completely against any race
condition there.
Thierry
