JBoss 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