[Oisf-devel] per thread pools

[Victor Julien]

On 04/26/2013 05:05 PM, Kenneth Steele wrote:
> One consideration for performance is keeping only one lock per cache-line to prevent false sharing. So in this case, an array of structures would be better.
> 
> struct AllocPool {
>     Pool tpools;
>     Lock tlocks;
> } __attribute__((aligned(64)));
> 
> struct AllocPool ThreadPools[threadnum];

Excellent point. In preparation of this I've added cache line size
detection to our autojunk:
https://github.com/inliniac/suricata/commit/724ad9e8e74a72543110f1a9414f10281a08d711

Cheers,
Victor


> -----Original Message-----
> From: oisf-devel-bounces at openinfosecfoundation.org [mailto:oisf-devel-bounces at openinfosecfoundation.org] On Behalf Of Victor Julien
> Sent: Thursday, April 25, 2013 5:16 AM
> To: Oisf-devel at openinfosecfoundation.org
> Subject: [Oisf-devel] per thread pools
> 
> We have several tickets for per thread pools [1,2,3]. Mostly for the stream engine, but there will be a need for other parts of the engine as well, like in my DNS parser work and Anoop's streaming parser API.
> 
> The challenge is that the per-thread pools should be accessible by other threads in some cases. Most importantly here is that our Flow manager thread cleans up Flows, including TCP ssn, app layer, etc. It will have to be able to return things to the proper pools.
> 
> Below is a simple proposal for a API.
> 
> In summary, we'd just use an array of Pools and an array of locks per pool. The index to the arrays will be a thread id. We can't use the OS thread id obviously as it would make the arrays much bigger than necessary.
> 
> Since most activity will be in the thread itself, contention should be low, certainly much lower than the current global pools. We still need the locking for the cases where another thread does access a pool.
> 
> 
> 1. We need a per thread idx, 0 - threadnum
> 
> 2. Thread pool defined as:
> 
> struct ThreadPool {
>     Pool tpools[threadnum];
>     Lock tlocks[threadnum];
> };
> 
> 3. A normal ThreadPoolGet(...)
> 
> void *ThreadPoolGet(ThreadPool *p) {
>     int tid = ThreadId();
>     Lock(p->tlocks[tid]);
>     void *ptr = PoolGet(tpools[tid]);
>     Unlock(p->tlocks[tid]);
>     return ptr;
> }
> 
> 4. A normal ThreadPoolReturn(...)
> 
> void ThreadPoolReturn(ThreadPool *p, void *ptr) {
>     int tid = ThreadId();
>     Lock(p->tlocks[tid]);
>     PoolReturn(tpools[tid], ptr);
>     Unlock(p->tlocks[tid]);
> }
> 
> 
> 5a. A ThreadPoolReturn(...) by another thread
> 
> void ThreadPoolReturnForId(ThreadPool *p, int tid, void *ptr) {
>     Lock(p->tlocks[tid]);
>     PoolReturn(tpools[tid], ptr);
>     Unlock(p->tlocks[tid]);
> }
> 
> An obvious problem is how in this last case the thread id should be stored. In stream-tcp-reassemble.c we use a variable in the TcpSegment structure (pool_size) to map the segment to a pool (we have pools for several segment sizes).
> 
> 5b. Maybe we can force the users of the pools to reserve 4 bytes at the start of their data:
> 
> struct TcpSegment {
>     ThreadPoolReserved res;
> 
>     ... segment vars...
> }
> 
> struct ThreadPoolReserved {
>     int id;
> }
> 
> void ThreadPoolReturnProbeId(ThreadPool *p, void *ptr) {
>     ThreadPoolReserved *r = ptr;
>     int tid = r->id;
>     Lock(p->tlocks[tid]);
>     PoolReturn(tpools[tid], ptr);
>     Unlock(p->tlocks[tid]);
> }
> 
> Thoughts?
> 
> Cheers,
> Victor
> 
> 
> [1] https://redmine.openinfosecfoundation.org/issues/519
> [2] https://redmine.openinfosecfoundation.org/issues/520
> [3] https://redmine.openinfosecfoundation.org/issues/521
> --
> ---------------------------------------------
> Victor Julien
> http://www.inliniac.net/
> PGP: http://www.inliniac.net/victorjulien.asc
> ---------------------------------------------
> 
> _______________________________________________
> Suricata IDS Devel mailing list: oisf-devel at openinfosecfoundation.org
> Site: http://suricata-ids.org | Participate: http://suricata-ids.org/participate/
> List: https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel
> Redmine: https://redmine.openinfosecfoundation.org/
> 


-- 
---------------------------------------------
Victor Julien
http://www.inliniac.net/
PGP: http://www.inliniac.net/victorjulien.asc
---------------------------------------------