harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.com>
Subject Re: svn commit: r479181 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/class_support/classloader.cpp
Date Sun, 26 Nov 2006 18:17:52 GMT


Gregory Shimansky wrote:
> On Sunday 26 November 2006 03:30 Geir Magnusson Jr. wrote:
>> 1) Why add the SuspendDisabledChecker if not using it?
>>
>> 2) exactly where did you add the assertion?  :)
> 
> It is a hidden assertion class :)

Oh, come on.  An assertion as a side effect?

> 
> Look at the file vm/vmcore/include/suspend_checker.h. There are 2 classes 
> SuspendEnabledChecker and SuspendDisabledChecker. When declared, such 
> variable in constructor checks for suspend status, in destructor it checks 
> that the status is the same. It is often convenient to write just this one 
> line to make sure that all returns from the function have the same suspend 
> status because local variable destructor is executed on every function exit.
> 
> In release, when assert is a noop, these constructors/destructors are 
> optimized into noop as well.
> 
> I saw that this function uses raw ManagedObject pointers. This is dangerous in 
> case when suspend is enabled (equal to GC being enabled) as the object may be 
> moved at any time. So I decided to add this assertion. If it fails some time 
> it will signal that this function is called in a wrong unsafe mode.

Ok - but don't you think that the assert() would be clearer and just as 
efficient?

geir

> 
>> gshimansky@apache.org wrote:
>>> Author: gshimansky
>>> Date: Sat Nov 25 11:59:38 2006
>>> New Revision: 479181
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=479181
>>> Log:
>>> Fixed 32-bitness in classloader tracing. Added assertion before using a
>>> raw heap object pointer
>>>
>>>
>>> Modified:
>>>    
>>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/class_support/classloader.cpp
>>>
>>> Modified:
>>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/class_support/classloader.cpp
>>> URL:
>>> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/c
>>> lass_support/classloader.cpp?view=diff&rev=479181&r1=479180&r2=479181
>>> =========================================================================
>>> ===== ---
>>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/class_support/classloader.cpp
>>> (original) +++
>>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/class_support/classloader.cpp
>>> Sat Nov 25 11:59:38 2006 @@ -589,11 +589,13 @@
>>>
>>>  ClassLoader* ClassLoader::AddClassLoader( ManagedObject* loader )
>>>  {
>>> +    SuspendDisabledChecker sdc;
>>> +
>>>      LMAutoUnlock aulock( &(ClassLoader::m_tableLock) );
>>>      ClassLoader* cl = new UserDefinedClassLoader();
>>>      TRACE2("classloader.unloading.add", "Adding class loader "
>>>          << cl << " (" << loader << " : "
>>> -        << ((VTable*)(*(unsigned**)(loader)))->clss->get_name()->bytes
>>> << ")"); +        << loader->vt()->clss->get_name()->bytes
<< ")");
>>>      cl->Initialize( loader );
>>>      if( m_capacity <= m_nextEntry )
>>>          ReallocateTable( m_capacity?(2*m_capacity):32 );
> 

Mime
View raw message