[jbosscache-dev] Notifier - performance optimization

[Mircea Markus]

Manik Surtani wrote:
> My comments below:
>
> On 20 May 2008, at 22:57, Mircea Markus wrote:
>
>> Hi,
>>
>> There is place for some performance optimization with respect to the 
>> notification mechanism, in the Notifier class:
>> 1) there are defensive Maps being created for being passed in the 
>> pre/post notifications, even if no listeners are registered.
>
> Are you looking at trunk?  I can see that the defensive copy is only 
> made *after* checking for whether a listener exists.
>
> E.g., in notifyNodeModified():
>
>           if (listeners != null && !listeners.isEmpty())
>           {
>              boolean originLocal = ctx.isOriginLocal();
>              Map dataCopy = copy(data, useMarshalledValueMaps);
>         ... etc ...
>     }
>
I am referring to the maps which are being passed to the notifier, not 
the one that are being build within the notifier itself.
E.g.PutDataMapCommand
      Map existingData = nodeSPI.getDataDirect();
      if (!existingData.isEmpty())
      {
         oldData = new HashMap(existingData); // defensive copy
      }

>> 2) even if we do not have listeners for a certain event, there is 
>> still a lookup in the listeners map to check for the available list 
>> of listeners
>
> Yes, you mean, for example,
>
>       List<ListenerInvocation> listeners = 
> listenerInvocations.get(NodeModified.class);
>
> in notifyNodeModified().
>
> Since we have a finite set of callbacks, I don't mind these lists 
> becoming members as you suggested.  The only thing is, they should be 
> final and never null, since they need to be threadsafe.  Constructing 
> them lazily will become a problem without synchronizations.
>
ok.
>> Optimizations:
>> for 1) - do not trigger a notification if no listener registered, use 
>> a guard - similar to log.isTraceEnabled()
>> for 2) - rather than keeping a map with listeners registered for each 
>> annotation type, rather keep the listeners list as a member - this 
>> way we'll skip the list lookup.
>>
>> Another thing that just come into my mind is not iterating on the 
>> list of listeners but rather use sequential access - small gain though.
>
> Well - looking at the implementation of a CopyOnWriteArrayList's 
> iterator, it is nothing but sequential access.  The only gain would be 
> saving the construction of the iterator object and in wrapping array 
> accesses in a method (iterator.next()).  For the sake of readability 
> though I'd rather stick with the foreach loop.
>
Haven't noticed that this is an CopyOnWriteArrayList, I agree with you 
about readability.
>> I expect this would gain us some performance, especially in the 
>> standalone mode.
>> WDYT?
>>
>> Cheers,
>> Mircea
>> _______________________________________________
>> jbosscache-dev mailing list
>> jbosscache-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/jbosscache-dev
>
> -- 
> Manik Surtani
> Lead, JBoss Cache
> manik at jboss.org
>
>
>
>
>
>