ChenHuajun / pg_roaringbitmap

RoaringBitmap extension for PostgreSQL
Apache License 2.0
218 stars 37 forks source link

v0.4.1~v0.5.0 memory leak and performance is 50% lower than v0.3.0 #9

Closed ChenHuajun closed 4 years ago

ChenHuajun commented 4 years ago

Problem

v0.4.1~v0.5.0 memory leak and performance is 50% lower than v0.3.0

Test SQL

DDL

CREATE TABLE bitmaptb4 AS
SELECT
    (
        SELECT
            rb_build_agg ((random() * 10000000)::int)
        FROM
            generate_series(1, 1000000) id) bitmap
FROM
    generate_series(
        1, 10000
);

Query SQL

select rb_cardinality(rb_or_agg(bitmap)) from bitmaptb4;

executed in v0.3.0

run time

app_db=# select rb_cardinality(rb_or_agg(bitmap)) from bitmaptb4;
 rb_cardinality 
----------------
         951606
(1 row)

Time: 6730.483 ms (00:06.730)

top -c

    PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND  
...
2961449 postgres  20   0 32.200g   9780   3744 R 100.0  0.0   0:31.99 postgres: admin app_db [local] SELECT

perf report

+   38.36%     0.00%  postgres  [unknown]          [.] 0x0000000000009dc1
+   38.19%     0.00%  postgres  [unknown]          [.] 0x00007f8948be9358
+   36.18%    36.06%  postgres  libc-2.17.so       [.] __memcpy_ssse3_back
+   14.75%     0.27%  postgres  roaringbitmap.so   [.] roaring_bitmap_or_inplace
+   14.48%    14.43%  postgres  roaringbitmap.so   [.] bitset_container_or
+   10.19%     0.00%  postgres  [unknown]          [.] 0x0040002800000000
...

executed in v0.5.0

run time

app_db=# select rb_cardinality(rb_or_agg(bitmap)) from bitmaptb4;
 rb_cardinality 
----------------
         951606
(1 row)

Time: 12873.381 ms (00:12.873)

top -c

    PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND  
2961106 postgres  20   0 68.349g 0.035t   4604 R 100.0 28.8   1:37.06 postgres: admin app_db [local] SELECT 

perf report

+   31.18%     0.27%  postgres  [kernel.kallsyms]  [k] do_page_fault 
+   30.81%     0.75%  postgres  [kernel.kallsyms]  [k] __do_page_fault   
+   29.62%     1.42%  postgres  [kernel.kallsyms]  [k] handle_mm_fault   
+   24.99%     1.74%  postgres  roaringbitmap.so   [.] bitset_container_clear    
+   23.61%     0.00%  postgres  [unknown]          [k] 0000000000000000  
+   18.99%     0.00%  postgres  [unknown]          [.] 0x0000000000009dc1        
+   18.87%     0.00%  postgres  [unknown]          [.] 0x00007f8948be8d58        
+   17.89%     0.84%  postgres  libc-2.17.so       [.] _int_malloc   
+   17.76%    17.63%  postgres  libc-2.17.so       [.] __memcpy_ssse3_back       
+   16.80%     0.27%  postgres  [kernel.kallsyms]  [k] alloc_pages_vma   
+   16.31%     0.71%  postgres  [kernel.kallsyms]  [k] __alloc_pages_nodemask    
+   10.84%    10.84%  postgres  [kernel.kallsyms]  [k] clear_page_c_e    
+    9.25%     0.00%  postgres  [unknown]          [k] 0x0000000c04210080        
...

Reason

The memory allocated in rb_or_trans () PG_GETARG_BYTEA_P () `in the context of aggctx memory was not released in time.

Datum rb_or_trans(PG_FUNCTION_ARGS) { ... oldcontext = MemoryContextSwitchTo(aggctx);

    bb = PG_GETARG_BYTEA_P(1); // memeory leak here!!!
    r2 = roaring_bitmap_portable_deserialize(VARDATA(bb));

    if (PG_ARGISNULL(0)) {
        r1 = r2;
    } else {
        r1 = (roaring_bitmap_t *) PG_GETARG_POINTER(0);
        roaring_bitmap_or_inplace(r1, r2);
        roaring_bitmap_free(r2);
    }

    MemoryContextSwitchTo(oldcontext);

... }

Repair

        oldcontext = MemoryContextSwitchTo(aggctx);

        bb = PG_GETARG_BYTEA_P(1);

=>

        bb = PG_GETARG_BYTEA_P(1);

        oldcontext = MemoryContextSwitchTo(aggctx);
ChenHuajun commented 4 years ago

Fixed by https://github.com/ChenHuajun/pg_roaringbitmap/commit/d14ee9fee73646b1dcb05ebfafe6df0ac212ac46