search

dragonflydb / dragonfly

A modern replacement for Redis and Memcached
https://www.dragonflydb.io/
Other
24.84k stars 894 forks source link

get rid of inline tables that use dynamic allocation #3287

Closed romange closed 2 weeks ago

romange commented 3 weeks ago

server/acl/acl_commands_def.h has bunch of tables allocated before main is even started.

I do not like it because it forces memory allocator to initialize before we even configure it. Please change the initialization code so that it would be called explicitly from, say, Service c-tor.

Also, please refer to the following guidelines regarding global variables:

[1] https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables [2] https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ri-global-init

On my development machine, the dragonfly process gets stuck in release mode in the following stack-trace:

(gdb) 
#0  0x0000555555b01c80 in _mi_malloc_generic (heap=0x5555564bc600 <_mi_heap_main>, size=800, zero=false, huge_alignment=0) at /home/roman/project/dragonfly/build-opt/third_party/mimalloc/include/mimalloc/internal.h:397
#1  0x0000555555af7e2f in _mi_heap_malloc_zero_ex (zero=false, huge_alignment=0, size=800, heap=0x555556448b40 <_mi_heap_empty>) at /home/roman/project/dragonfly/build-opt/third_party/mimalloc/src/alloc.c:163
#2  _mi_heap_malloc_zero (zero=false, size=800, heap=0x555556448b40 <_mi_heap_empty>) at /home/roman/project/dragonfly/build-opt/third_party/mimalloc/src/alloc.c:181
#3  mi_heap_malloc (size=800, heap=0x555556448b40 <_mi_heap_empty>) at /home/roman/project/dragonfly/build-opt/third_party/mimalloc/src/alloc.c:185
#4  mi_heap_alloc_new (size=800, heap=0x555556448b40 <_mi_heap_empty>) at /home/roman/project/dragonfly/build-opt/third_party/mimalloc/src/alloc.c:511
#5  mi_new (size=800) at /home/roman/project/dragonfly/build-opt/third_party/mimalloc/src/alloc.c:517
#6  0x0000555555682244 in operator new (n=800) at /home/roman/project/dragonfly/src/core/allocation_tracker.h:73
#7  0x00005555556867ad in __gnu_cxx::new_allocator<absl::lts_20240116::container_internal::AlignedType<8ul> >::allocate (this=<synthetic pointer>, __n=<optimized out>) at /usr/include/c++/11/ext/new_allocator.h:127
#8  std::allocator<absl::lts_20240116::container_internal::AlignedType<8ul> >::allocate (__n=<optimized out>, this=<synthetic pointer>) at /usr/include/c++/11/bits/allocator.h:185
#9  std::allocator_traits<std::allocator<absl::lts_20240116::container_internal::AlignedType<8ul> > >::allocate (__n=<optimized out>, __a=<synthetic pointer>...) at /usr/include/c++/11/bits/alloc_traits.h:464
#10 absl::lts_20240116::container_internal::Allocate<8ul, std::allocator<char> > (n=<optimized out>, alloc=<optimized out>) at /home/roman/project/dragonfly/build-opt/_deps/abseil_cpp-src/absl/container/internal/container_memory.h:65
#11 absl::lts_20240116::container_internal::HashSetResizeHelper::InitializeSlots<std::allocator<char>, 24ul, true, 8ul> (this=this@entry=0x7fffffffdd00, c=..., old_slots=old_slots@entry=0x0, alloc=...)
    at /home/roman/project/dragonfly/build-opt/_deps/abseil_cpp-src/absl/container/internal/raw_hash_set.h:1615
#12 0x0000555555689904 in absl::lts_20240116::container_internal::raw_hash_set<absl::lts_20240116::container_internal::FlatHashMapPolicy<std::basic_string_view<char, std::char_traits<char> >, unsigned int>, absl::lts_20240116::container_internal::StringHash, absl::lts_20240116::container_internal::StringEq, std::allocator<std::pair<std::basic_string_view<char, std::char_traits<char> > const, unsigned int> > >::resize (
    this=0x5555564c5300 <dfly::acl::CATEGORY_INDEX_TABLE>, new_capacity=<optimized out>) at /home/roman/project/dragonfly/build-opt/_deps/abseil_cpp-src/absl/container/internal/compressed_tuple.h:92
#13 0x0000555555689ee6 in absl::lts_20240116::container_internal::raw_hash_set<absl::lts_20240116::container_internal::FlatHashMapPolicy<std::basic_string_view<char, std::char_traits<char> >, unsigned int>, absl::lts_20240116::container_internal::StringHash, absl::lts_20240116::container_internal::StringEq, std::allocator<std::pair<std::basic_string_view<char, std::char_traits<char> > const, unsigned int> > >::raw_hash_set<std::pair<std::basic_string_view<char, std::char_traits<char> >, unsigned int> const*> (this=0x5555564c5300 <dfly::acl::CATEGORY_INDEX_TABLE>, first=0x7fffffffde20, last=0x7fffffffe0a8, bucket_count=<optimized out>, hash=..., eq=..., alloc=...)
    at /home/roman/project/dragonfly/build-opt/_deps/abseil_cpp-src/absl/container/internal/raw_hash_set.h:2115
#14 0x000055555565deb2 in absl::lts_20240116::container_internal::raw_hash_set<absl::lts_20240116::container_internal::FlatHashMapPolicy<std::basic_string_view<char, std::char_traits<char> >, unsigned int>, absl::lts_20240116::container_internal::StringHash, absl::lts_20240116::container_internal::StringEq, std::allocator<std::pair<std::basic_string_view<char, std::char_traits<char> > const, unsigned int> > >::raw_hash_set (alloc=..., eq=..., hash=..., bucket_count=0, 
    init=..., this=0x5555564c5300 <dfly::acl::CATEGORY_INDEX_TABLE>) at /usr/include/c++/11/initializer_list:75
#15 absl::lts_20240116::container_internal::raw_hash_map<absl::lts_20240116::container_internal::FlatHashMapPolicy<std::basic_string_view<char, std::char_traits<char> >, unsigned int>, absl::lts_20240116::container_internal::StringHash, absl::lts_20240116::container_internal::StringEq, std::allocator<std::pair<std::basic_string_view<char, std::char_traits<char> > const, unsigned int> > >::raw_hash_set (this=0x5555564c5300 <dfly::acl::CATEGORY_INDEX_TABLE>)
    at /home/roman/project/dragonfly/build-opt/_deps/abseil_cpp-src/absl/container/internal/raw_hash_map.h:65
#16 absl::lts_20240116::flat_hash_map<std::basic_string_view<char, std::char_traits<char> >, unsigned int, absl::lts_20240116::container_internal::StringHash, absl::lts_20240116::container_internal::StringEq, std::allocator<std::pair<std::basic_string_view<char, std::char_traits<char> > const, unsigned int> > >::raw_hash_set (this=0x5555564c5300 <dfly::acl::CATEGORY_INDEX_TABLE>)
kostasrim commented 3 weeks ago

@romange on it!