<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I've gone through a few iterations, some using limited size arrays,<br>some using enum values, and this is the final result that I am<br>actually quite satisfied with. I know it *looks* like it is<br>spamming on dynamic allocation and list iteration but consider:</p><ul><li>in osmo-msc I actually avoid all dynamic allocation using static</li><li>use count entries (see in-code comment below, use_count.h:174).</li></ul><ul><li>use count lists so far have at most six entries in practice,</li><li>usually more like 3. Traversing six llist->next pointers is less</li><li>effort than doing your common strlen(); compare with osmo_strlcpy()</li><li>which always does a strlen() no matter what, and we don't think</li><li>twice.</li></ul></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">"Premature Optimization is the root of all evil". But SCNR:  The expensive<br>cost about linked list iteration is not CPU cost, it's cache line misses. Your<br>CPU is likely going to spend thousands of cycles doing nothing just waiting<br>for data from RAM to arrive, and in the worst case for each of the list elements<br>you iterate over, as they are not contiguously allocated and spread across RAM.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm not saying this level of optimization is important here, I'm just sharing<br>some perspective.  The kernel has some explicit prefetch macros in the linked<br>list code to improve the situation, not sure how much.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><ul><li>Compared to get_value_string() which we do *all* *the* *time* (at</li><li>least when logging is enabled), this API imposes negligible</li><li>traversal overhead.</li></ul></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">yes.  wireshark has an optimzied get_value_string derivative now, which<br>generates a hash table at runtime.  We could switch to that if it ever<br>was an issue. Debug logging is not enabled in typical operation, though.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><br> > - Let alone comparing to ran connection or VLR subscriber or<br> > gsm_trans lookup: so far we keep all gsm_trans items for all<br> > subscribers in *one single global* llist, and when looking for a<br> > trans for a given subscriber, we more often than not traverse the<br> > entire list. </pre><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, but that's rather easy to replace with a hash table or the like,<br>(Linux kernel has plenty of hash tables where the individual buckets<br>are llist_entry) where only few places will need to be changed.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><br> > I can understand that this *seems* quite wasteful at first, but if<br> > you take a closer look I expect that you guys will agree with me:<br> > it is a fairly small overhead for a very nice API.</pre><p style="white-space: pre-wrap; word-wrap: break-word;">Contrary to some of the other things we do so far, I think with this new<br>infrastructure it may not be as easy to optimize the implementation without<br>breaking the users.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'll read the patch one more time and provide more review.  I'm not fundamentally<br>opposed and if you think it's what you need, so be it.</p><p><a href="https://gerrit.osmocom.org/13121">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13121">change 13121</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13121"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ife31e6798b4e728a23913179e346552a7dd338c0 </div>
<div style="display:none"> Gerrit-Change-Number: 13121 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 07 Mar 2019 17:28:51 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-HasLabels: No </div>