[Asterisk-code-review] threadpool: Eliminate pointless AO2 usage. (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/10470 )

Change subject: threadpool: Eliminate pointless AO2 usage.
......................................................................


Patch Set 1:

> I would rather leave these items as ao2 objects.  They certainly
 > don't need the lock and you have identified some other
 > improvements.
 > 
 > The beauty of keeping them ao2 objects is you don't need to worry
 > about calling an appropriate destructor function if there even is
 > one when the ref is decremented.

Personally I disagree, these objects are so limited in scope that using AO2 for them seems similar to using RAII_VAR in a scope with only one or two escapes.  Even without locking the private part of the AO2 object would be 50% larger than the user_data (on x86_64 anyways).  On my system these objects currently have 16 bytes user data, 64 bytes AO2 private data (with AO2_DEBUG disabled).  Replacing these with NOLOCK AO2 objects would still more than double the size and cause the user_data to always cross a 32-byte boundary.

As for accidentally using the wrong cleanup method, I think that's the responsibility of reviewers to catch this.  I'd expect anyone making/reviewing changes to threadpool.c to give attention to details such as this.  If I added a '\note' to the doxygen of each allocation routine saying how to free would that make you OK with using ast_malloc instead of ao2?


-- 
To view, visit https://gerrit.asterisk.org/10470
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I2204d2615d9d952670fcb48e0a9c0dd1a6ba5036
Gerrit-Change-Number: 10470
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Sat, 13 Oct 2018 10:42:59 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181013/117a1bfd/attachment.html>