[jboss-user] [JBoss Microcontainer Development] New message: "Re: Profiling the dependency project"
Kabir Khan
do-not-reply at jboss.com
Fri Feb 19 10:48:15 EST 2010
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
More information about the jboss-user
mailing list