-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3513/
-----------------------------------------------------------

(Updated June 20, 2014, 1:42 p.m.)


Status
------

This change has been discarded.


Review request for Asterisk Developers, Joshua Colp and rmudgett.


Repository: Asterisk


Description
-------

Add weak reference capability to hash and list astobj2 containers.

Current (Strong-reference) behavior:  A container (list, hash or rbtree) 
increments an object's ref count when linked and decrements an object's ref 
count when unlinked.  Therefore the object can never be destroyed while it is 
an entry in a container unless someone accidentally decrements the object's ref 
count too many times.  In this case, the object is invalid but the container 
doesn't know it.  

Weak-reference behavior:  A container (list or hash) allocated with the option 
AO2_CONTAINER_ALLOC_OPT_WEAK_REF neither increments an object's ref count when 
linked nor decrements an object's ref count when unlinked.  The object's ref 
count can therefore validly reach 0 while still in a weak-ref container in 
which case the object is automatically and cleanly removed from any weak-ref 
container it may be in.  Because of the complexity of the red-black tree 
implementation, it isn't eligible for weak-reference behavior.

Use case:  The possible need for weak-ref containers came up during the 
development of the Sorcery registry.  The first call to sorcery_open from a 
module should create a new sorcery instance and subsequent calls from that same 
module should use that instance.  The last call to close should free that 
instance.  With a strong-ref container however, the container itself always has 
a reference to the instance so it doesn't get destroyed without special code to 
check the ref count on each call to close.  The same thing happens with the 
pjsip cli command registry.

Implementation:  astobj2.__priv_data now has a linked list that contains the 
weak-ref container nodes that reference the object.  When an object is added to 
a weak-ref container, the container node is added to the list instead of the 
node incrementing the object's ref count.  Similarly, when an object is removed 
from a weak-ref container the node is removed from the linked list instead of 
the object's ref count being decremented.  If an object's ref count reaches 0 
the object's linked list is traversed and the corresponding nodes are cleared 
effectively removing the object from the container.  NOTE:  An object's ref 
count is still incremented as the result of a retrieval (find, iterate, etc) 
from a weak-ref container.

Backwards compatibility:  Unless the AO2_CONTAINER_ALLOC_OPT_WEAK_REF flag was 
set on container allocation, all container operations remain exactly as they 
are today and nothing prevents an object from being a member of both strong and 
weak ref containers at the same time.

Performance implications:  Due to code reorganization, the performance of 
strong-ref containers is actually microscopically BETTER than the unmodified 
code and the performance of weak-ref containers is even better than that.  In 
other words, the performance of the default behavior was not penalized by the 
addition of the new feature.  The test framework now has a /main/astobj2/perf/ 
category to show relative performance.  NOTE:  Previously, when the test 
framework was enabled AO2_DEBUG was also enabled but this was skewing the 
results so I removed that auto-enablement of AO2_DEBUG.  The test framework 
never needed it.

The original RFC review request was here... 
https://reviewboard.asterisk.org/r/3382/
The major changes from the RFC are that a spinlock was added around the 
object's ref-counter operations and the rbtree's are now excluded from being 
weak-reference containers.


Diffs
-----

  branches/12/tests/test_astobj2_thrash.c 413157 
  branches/12/tests/test_astobj2.c 413157 
  branches/12/main/astobj2.c 413157 
  branches/12/include/asterisk/astobj2.h 413157 

Diff: https://reviewboard.asterisk.org/r/3513/diff/


Testing
-------

7 new tests were added to the existing astobj2 test framework.  All tests pass.

The testsuite is a little more challenging but I at least made sure that all 
the test that passed before the change still passed after the change.


Thanks,

George Joseph

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to