duplicate calls to addWeakRef /removeWeakRef in platform code

242 views
Skip to first unread message

devi prasad

unread,
Jun 18, 2009, 9:45:49 AM6/18/09
to android-...@googlegroups.com
While trying to understand the strong pointer and weak pointer
implementation, I found duplicate calls to 'addWeakRef' and
'removeWeakRef' in the 'RefBase' implementation. I've reproduced a
part of the code for quick reference:

Function 'incStrong' and 'incWeak' both call 'addWeakRef' which is redundant.
void RefBase::incStrong(const void* id) const
{
   weakref_impl* const refs = mRefs;
   refs->addWeakRef(id);
   refs->incWeak(id);
   ...
}

void RefBase::weakref_type::incWeak(const void* id)
{
   weakref_impl* const impl = static_cast<weakref_impl*>(this);
   impl->addWeakRef(id);
   ...
}

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

Functions 'decStrong' and 'decWeak' invoke 'removeWeakRef' which again
is redundant.
void RefBase::decStrong(const void* id) const
{
weakref_impl* const refs = mRefs;
...
refs->removeWeakRef(id);
refs->decWeak(id);
}

void RefBase::weakref_type::decWeak(const void* id)
{
weakref_impl* const impl = static_cast<weakref_impl*>(this);
impl->removeWeakRef(id);
....
}
----------------------------------------------------------------------------------------------

Of course, there's no harm in these duplicate calls since these
functions are NOPs when DEBUG_REFS is 0.

Yet, these calls could be avoided.

Dianne Hackborn

unread,
Jun 18, 2009, 3:30:48 PM6/18/09
to android-...@googlegroups.com
As you say, when compiled with debugging turned off they don't do anything (and thus get stripped out).  They shouldn't be removed, because they are there to implement the debugging code that the macro turns on.
--
Dianne Hackborn
Android framework engineer
hac...@android.com

Note: please don't send private questions to me, as I don't have time to provide private support, and so won't reply to such e-mails.  All such questions should be posted on public forums, where I and others can see and answer them.

devi prasad

unread,
Jun 18, 2009, 10:45:11 PM6/18/09
to android-...@googlegroups.com
Hi, Dianne, thanks for the note.However, I think I wasn't clear enough in my statement. What I meant is that both caller and called functions make a 'addWeakRef(..).' 

RefBase::incStrong(..) does a addWeakRef(..) before calling  RefBase::weakref_type::incWeak(..) which in turn does a addWeakRef(..).

I think we can remove these duplicate calls in the caller and called functions. This may save a few CPU cycles even in a debugging code.

Dianne Hackborn

unread,
Jun 18, 2009, 11:05:42 PM6/18/09
to android-...@googlegroups.com
Ah I see.  Well the debug code may be broken, though I wouldn't leap to that conclusion without actually testing it; I know it has been used in the past.  Anyway, these things get compiled out in non-debug builds, so there is no reason to change them unless you are actually trying to use this debug code and it isn't working and need to do this to fix it.
Reply all
Reply to author
Forward
0 new messages