[concurrency-interest] Thread subclass check locking is a bottleneck

[Gregg Wonderly]

Gregg Wonderly wrote:
> David Holmes wrote:
>> The "table" is an unsynchronized data structure hence it has to be 
>> protected
>> by a global lock of some form. I don't see any reason to synchronize 
>> on the
>> Class object as we're not updating state in the Class.

We are validating the usability of that class and writing that into the 
softCache.  The comment below,

 > I guess the question is whether it's safe to perform the audit on the
 > same class, simultaneously.

goes back to locking on the Class.  The data structure needs to be queried, some 
action performed, and then the result of that action put into the table.  The 
first and last steps do require synchronized access to the concurrently updated 
table.  However the validation step may only require the step to be performed 
exactly once, and/or not concurrently with another thread.  So that's why I'm 
suggesting locking on the class.

I was inferring the lock on cl to be in a context like the following

private static boolean isCCLOverridden(Class cl) {
     if (cl == Thread.class)
         return false;
     Boolean result = null;
     synchronized (subclassAudits) {
         result = (Boolean) subclassAudits.get(cl);
     }

     if (result == null) {
         synchronized( cl ) {
             /*
             * Note: only new Boolean instances (i.e., not Boolean.TRUE or
             * Boolean.FALSE) must be used as cache values, otherwise cache
             * entry will pin associated class.
             */
             result = new Boolean(auditSubclass(cl));
         }
         synchronized (subclassAudits) {
	    Boolean nresult;
             if( (nresult = subclassAudits.get(cl)) == null )
                 subclassAudits.put(cl, result);
	    else
		result = nresult;
         }
     }

     return result.booleanValue();
}

this looks very much like putIfAbsent and exactly the locking perspective that 
would occur with the use of ConcurrentHashMap.  The only issue is whether we 
want to do the locking on Class while the audit occurs.

Gregg Wonderly

>> It would certainly be better to have some kind of concurrently accessible
>> structure - especially as it is called twice per construction (once for
>> current class, once for parent class). If the SoftCache were a concurrent
>> map then I don't think any synchronization would be needed
> 
> I guess the question is whether it's safe to perform the audit on the 
> same class, simultaneously.  If that's okay, then I agree that using a 
> concurrent map would be more in line with the fact that this is a 
> concurrently accessed bit of code that seems to have a highly contested 
> (in my application) lock.
> 
> Gregg Wonderly
> 
>> David Holmes
>>
>>> -----Original Message-----
>>> From: concurrency-interest-bounces at cs.oswego.edu
>>> [mailto:concurrency-interest-bounces at cs.oswego.edu]On Behalf Of Gregg
>>> Wonderly
>>> Sent: Tuesday, 17 February 2009 3:33 AM
>>> To: concurrency-interest
>>> Subject: [concurrency-interest] Thread subclass check locking is a
>>> bottleneck
>>>
>>>
>>> I have an old application that implements it's own thread pooling.  But,
>>> this is an issue simply related to the java.lang.Thread constructor
>>> locking on subclass construction.  I am seeing stuck threads of the 
>>> form:
>>>
>>>    java.lang.Thread.State: BLOCKED (on object monitor)
>>>         at java.lang.Thread.isCCLOverridden(Thread.java:1521)
>>>         - waiting to lock <0xaca0a778> (a sun.misc.SoftCache)
>>>         at java.lang.Thread.init(Thread.java:338)
>>>         at java.lang.Thread.<init>(Thread.java:419)
>>>
>>> The global lock used there seems to be an unfortunate choice.  The code
>>> I see on the internet is:
>>>
>>>  1615       private static boolean isCCLOverridden(Class cl) {
>>>  1616           if (cl == Thread.class)
>>>  1617               return false;
>>>  1618           Boolean result = null;
>>>  1619           synchronized (subclassAudits) {
>>>  1620               result = (Boolean) subclassAudits.get(cl);
>>>  1621               if (result == null) {
>>>  1622                   /*
>>>  1623                    * Note: only new Boolean instances
>>> (i.e., not Boolean.TRUE or
>>>  1624                    * Boolean.FALSE) must be used as cache
>>> values, otherwise cache
>>>  1625                    * entry will pin associated class.
>>>  1626                    */
>>>  1627                   result = new Boolean(auditSubclass(cl));
>>>  1628                   subclassAudits.put(cl, result);
>>>  1629               }
>>>  1630           }
>>>  1631           return result.booleanValue();
>>>  1632       }
>>>
>>> Wouldn't it be more prudent for the lock held during
>>> auditSubclass to be on cl and not the global table?
>>>
>>> Gregg Wonderly
>>>
>>>
>>> _______________________________________________
>>> Concurrency-interest mailing list
>>> Concurrency-interest at cs.oswego.edu
>>> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>
>>
>>
> 
> 
>