net: Add buffer pool to replace connection allocation #10
Conversation
|
|
|
Already merged as apache/nuttx#15285 |
Our net socket connection allocations are powerful but redundant because they're implemented once in each protocol. This is not good for further optimizing and extending to other allocations, so maybe we can add a common implementation for the usage. Impact: 1. We add a `struct net_bufpool_s` as pool descriptor, which may use a little bit more memory than previous implementation (~28Bytes). 2. We share same functions between pools, so code size may shrink under some scenarios. Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
To allow dynamic allocation of these structs, performance keeps the same as the previous implementation. Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
wengzhe
force-pushed
the
openvela-net
branch
from
a0e1499 to
6eb664a
Compare
|
|
||
| /* If we get here, then we didn't exceed maxalloc. */ | ||
|
|
||
| if (pool->dynalloc > 0 && sq_peek(&pool->freebuffers) == NULL) |
There was a problem hiding this comment.
should we hold lock before accessing freebuffers?
There was a problem hiding this comment.
Currently, the bufpool is not protecting itself and the caller holds the lock(e.g. g_free_lock or net_lock) if the caller wants to use it concurrently. This patch also helps us find out that some protocol uses dedicated lock while others use net lock, which we may optimize later.
There was a problem hiding this comment.
should move g_free_lock to mem pool
There was a problem hiding this comment.
It's not easy to do this, take UDP as an example, UDP's g_free_lock protects not only the conn alloc, but also other list like g_active_udp_connections, but active list is not common for all protocols/buffers. So I just put the lock optimization into the work item, replacing the wrb optimization.
BTW, we don't really need to make sure every data structure is thread-safe (e.g. queues, hashtables, etc.), but yes, it will perform better if we lock inside instead of lock by caller.
| @@ -89,6 +116,26 @@ enum tv2ds_remainder_e | |||
| TV2DS_CEIL /* Force to next larger full decisecond */ | |||
| }; | |||
|
|
|||
| /* This structure is used to manage a pool of network buffers */ | |||
|
|
|||
| struct net_bufpool_s | |||
There was a problem hiding this comment.
it's useful, so we move to mm folder?
There was a problem hiding this comment.
Yes, we can move it anytime because we are always using NET_BUFPOOL_* macros.
Summary
Patches included:
Our net socket connection allocations are powerful but redundant because they're implemented once in each protocol. This is not good for further optimizing and extending to other allocations, so maybe we can add a common implementation for the usage.
Impact
struct net_bufpool_sas pool descriptor, which may use a little bit more memory than previous implementation (~28Bytes).Testing
Mainly on SIM.