[jboss-user] [JBoss Microcontainer Development] New message: "Re: Profiling the dependency project"

[Kabir Khan]

User development,

A new message was posted in the thread "Profiling the dependency project":

http://community.jboss.org/message/527292#527292

Author  : Kabir Khan
Profile : http://community.jboss.org/people/kabir.khan@jboss.com

Message:
--------------------------------------------------------------
> alesj wrote:
>  
> 
> > Since installCallbacks and uninstallCallbacks are ConcurrentHashMaps, I think the read lock is unnecessary here in AbstractController:
> Since I think callbacks are already used in some explicitly locked code (via Lock),
> what about if we leave the locks and just change the callbacks to plain HashMap?
>  
> Like discussed on the call, what is faster in a really-rare-concurrent env?
> * lock + plain hash collection
> * concurrent hash collection
>  
> Jason, DML?
I see we are adding them here (and similarly for removing them)
 
   protected <T> void addCallback(Object name, boolean isInstallPhase, CallbackItem<T> callback)
   {
      lockWrite();
      try
      {
         Map<Object, Set<CallbackItem<?>>> map = (isInstallPhase ? installCallbacks : uninstallCallbacks);
         Set<CallbackItem<?>> callbacks = map.get(name);
         if (callbacks == null)
         {
            callbacks = new HashSet<CallbackItem<?>>();
            map.put(name, callbacks);
         }
         callbacks.add(callback);
      }
      finally
      {
         unlockWrite();
      }
   }
 

 
So this needs some rethinking. Since callbacks is a plain HashSet and the user of getCallbacks()  is iterating over them this code is broken.
 
So we either make everything plain HashMap/Set, reintroduce the locks, and replace getContexts() with this method where we do the iteration (via addAll()) with the lock taken:
 
   /**
    * Get the matching callbacks.
    *
    * @param result the set to put any callbacks into
    * @param name demand name
    * @param isInstallPhase install or uninstall phase
    * @return The passed in result parameter. If it was null and callbacks were found a new set is created 
    */
   protected Set<CallbackItem<?>> addCallbacks(Set<CallbackItem<?>> result, Object name, boolean isInstallPhase)
   {
      lockRead();
      try
      {
         Map<Object, Set<CallbackItem<?>>> map = (isInstallPhase ? installCallbacks : uninstallCallbacks);
         Set<CallbackItem<?>> callbacks = map.get(name);
         if (callbacks != null)
         {
            if (result == null)
               result = new HashSet<CallbackItem<?>>();
            result.addAll(callbacks);
         }
      }
      finally
      {
         unlockRead();
      }
      return result;
   }

 
Or we make everything involved concurrent map set that would work too. I need to understand a bit better how this code is used.

--------------------------------------------------------------

To reply to this message visit the message page: http://community.jboss.org/message/527292#527292