Conversation
antonio2368
left a comment
There was a problem hiding this comment.
LGTM but I'm interested in integration with CH
| /// change state between active/dirty/neither. | ||
| /// Indended use is to subtract the values before and after an alloc/free call. | ||
| /// Absolute values are not meaningful. | ||
| int64_t active_bytes_delta; // total size of active pages (i.e. containing any live allocations) |
There was a problem hiding this comment.
| int64_t active_bytes_delta; // total size of active pages (i.e. containing any live allocations) | |
| int64_t active_bytes_delta; // total size of active pages (i.e. containing any live allocations) |
|
After looking more at the MemoryTracker-adjacent code, I see what you mean, this wouldn't be easy to fit in there... Also noticed that kernel memory usage is not negligible (e.g. in a small local test I saw 4.7 GB out of 8.3 GB being used by sockets, "sock" in |
|
(Abandoning for now, can revive from the list of PRs later if needed. This PR seems good, but the integration with MemoryTracker et al would be a little complicated.) |
We tried to load `g` from `bitmap[i]` before checking it is actually a
valid load. Tweaked a loop a bit to `break` early, when we are done
scanning for bits.
Before this commit undefined behaviour sanitizer from GCC 14+ was
unhappy at `test/unit/bitmap` test with following error.
```
../include/jemalloc/internal/bitmap.h:293:5: runtime error: load of
address 0x7bb1c2e08008 with insufficient space for an object of type
'const bitmap_t'
<...>
#0 0x62671a149954 in bitmap_ffu ../include/jemalloc/internal/bitmap.h:293
#1 0x62671a149954 in test_bitmap_xfu_body ../test/unit/bitmap.c:275
#2 0x62671a14b767 in test_bitmap_xfu ../test/unit/bitmap.c:323
#3 0x62671a376ad1 in p_test_impl ../test/src/test.c:149
#4 0x62671a377135 in p_test ../test/src/test.c:200
jemalloc#5 0x62671a13da06 in main ../test/unit/bitmap.c:336
<...>
```
This commit allows to enable sanitizers with autoconf options, instead
of modifying `CFLAGS`, `CXXFLAGS` and `LDFLAGS` directly.
* `--enable-tsan` option to enable Thread Sanitizer.
* `--enable-ubsan` option to enable Undefined Behaviour Sanitizer.
End goal is to speedup development by finding problems quickly, early
and easier. Eventually, when all current issues will be fixed, we can
enable sanitizers in CI. Fortunately, there are not a lot of problems we
need to fix.
Address Sanitizer is a bit controversial, because it replaces memory
allocator, so we decided to left it out for a while.
Below are couple of examples of how tests look like under different
sanitizers at the moment.
```
$ ../configure --enable-tsan --enable-debug
<...>
asan : 0
tsan : 1
ubsan : 0
$ make -j`nproc` check
<...>
Thread T13 (tid=332043, running) created by main thread at:
#0 pthread_create <null> (libtsan.so.0+0x61748)
#1 thd_create ../test/src/thd.c:25 (bin_batching+0x5631ca)
#2 stress_run ../test/unit/bin_batching.c:148
(bin_batching+0x40364c)
#3 test_races ../test/unit/bin_batching.c:249
(bin_batching+0x403d79)
#4 p_test_impl ../test/src/test.c:149 (bin_batching+0x562811)
jemalloc#5 p_test_no_reentrancy ../test/src/test.c:213
(bin_batching+0x562d35)
jemalloc#6 main ../test/unit/bin_batching.c:268 (bin_batching+0x40417e)
SUMMARY: ThreadSanitizer: data race
../include/jemalloc/internal/edata.h:498 in edata_nfree_inc
```
```
$ ../configure --enable-ubsan --enable-debug
<...>
asan : 0
tsan : 0
ubsan : 1
$ make -j`nproc` check
<...>
=== test/unit/hash ===
../test/unit/hash.c:119:16: runtime error: left shift of 176 by 24
places cannot be represented in type 'int'
<...>
```
Here's a way to do (hopefully) solid memory limiting using the smallest amount of jemalloc changes.
(I spent a while following the code paths and am pretty sure these are the only code sites that need to be updated, as long as hugepage allocator ("hpa" option in jemalloc config) is disabled.)
(Tested it a little, and the
je_clickhouse_resident_bytesandje_clickhouse_active_bytesseem accurate, ~2-15% above RssAnon.)