danielaparker / jsoncons

A C++, header-only library for constructing JSON and JSON-like data formats, with JSON Pointer, JSON Patch, JSON Schema, JSONPath, JMESPath, CSV, MessagePack, CBOR, BSON, UBJSON
https://danielaparker.github.io/jsoncons
Other
726 stars 164 forks source link

`sorted_json_object` doesn't (always?) pass allocator to `members_` #485

Closed chakaz closed 9 months ago

chakaz commented 10 months ago

I created this small patch: https://github.com/dragonflydb/jsoncons/pull/1/files But it only handles a couple of cases, but I think that generally any attempt to insert to members_ should be passing an allocator into it, no?

danielaparker commented 10 months ago

I think jsoncons is correct here.

jsoncons only supports propagating stateful allocators, including std::pmr::polymorphic_allocator, or a non-propagating stateful allocator wrapped in a std::scoped_allocator_adaptor. Therefore the allocator is automatically propagated in the emplace_back call to a constructor of key_value.

I believe it would be this constructor that would be called:

        template <typename... Args>
        key_value(const key_type& name,  Args&& ... args, const allocator_type& alloc)
            : key_(name, alloc), value_(std::forward<Args>(args)..., alloc)
        {
        }
chakaz commented 9 months ago

Thank @danielaparker for the quick reply. I must say that the world of std allocators is new to me, so I'm sorry for filing this issue prior to understanding allocator propagation. Since your response, I've been learning about it. I must say that it's quite a rabbit hole :) Anyway, the constructor you pasted above is in fact not called, at least not in our use case, and I (think I?) was able to figure out why.

Our code looks like this:

  using JsonType = jsoncons::pmr::json;

  json_decoder<JsonType> decoder(
      std::pmr::polymorphic_allocator<char>{CompactObj::memory_resource()});
  basic_json_parser<char, std::pmr::polymorphic_allocator<char>> parser(
      basic_json_decode_options<char>{}, JsonErrorHandler,
      std::pmr::polymorphic_allocator<char>{CompactObj::memory_resource()});

  parser.update(input);
  parser.finish_parse(decoder, ec);

Our call stack looks like this:

operator new(std::size_t n, std::align_val_t al) (/home/shahar/dragonfly/src/core/allocation_tracker.h:134)
std::pmr::memory_resource::allocate(std::pmr::memory_resource * const this, std::size_t __bytes, std::size_t __alignment) (/usr/include/c++/11/memory_resource:106)
std::pmr::polymorphic_allocator<char>::allocate(std::pmr::polymorphic_allocator<char> * const this, std::size_t __n) (/usr/include/c++/11/memory_resource:179)
std::allocator_traits<std::pmr::polymorphic_allocator<char> >::allocate(std::pmr::polymorphic_allocator<char> & __a, std::allocator_traits<std::pmr::polymorphic_allocator<char> >::size_type __n) (/usr/include/c++/11/bits/alloc_traits.h:318)
std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::_M_create(std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> > * const this, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::size_type & __capacity, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::size_type __old_capacity) (/usr/include/c++/11/bits/basic_string.tcc:153)
std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::_M_construct<char*>(std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> > * const this, char * __beg, char * __end) (/usr/include/c++/11/bits/basic_string.tcc:219)
std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::_M_construct_aux<char*>(std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> > * const this, char * __beg, char * __end) (/usr/include/c++/11/bits/basic_string.h:255)
std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::_M_construct<char*>(std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> > * const this, char * __beg, char * __end) (/usr/include/c++/11/bits/basic_string.h:274)
std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> > * const this, const std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> > & __str) (/usr/include/c++/11/bits/basic_string.h:459)
jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >::key_value<jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > const&>(jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * const this, const jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >::key_type & name) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_object.hpp:99)
std::construct_at<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > const&>(jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * __location) (/usr/include/c++/11/bits/stl_construct.h:97)
_ZZSt39uninitialized_construct_using_allocatorIN8jsoncons9key_valueINSt7__cxx1112basic_stringIcSt11char_traitsIcENSt3pmr21polymorphic_allocatorIcEEEENS0_10basic_jsonIcNS0_13sorted_policyES8_EEEENS7_ISD_EEJRS9_SC_EEPT_SH_RKT0_DpOT1_ENKUlDpOT_E_clIJSF_SC_RKSE_EEEDaSQ_(const struct {...} * const __closure) (/usr/include/c++/11/bits/uses_allocator_args.h:208)
_ZSt13__invoke_implIPN8jsoncons9key_valueINSt7__cxx1112basic_stringIcSt11char_traitsIcENSt3pmr21polymorphic_allocatorIcEEEENS0_10basic_jsonIcNS0_13sorted_policyES8_EEEEZSt39uninitialized_construct_using_allocatorISD_NS7_ISD_EEJRS9_SC_EEPT_SJ_RKT0_DpOT1_EUlDpOT_E_JSH_SC_RKSG_EESI_St14__invoke_otherOSK_SP_(struct {...} && __f) (/usr/include/c++/11/bits/invoke.h:61)
_ZSt8__invokeIZSt39uninitialized_construct_using_allocatorIN8jsoncons9key_valueINSt7__cxx1112basic_stringIcSt11char_traitsIcENSt3pmr21polymorphic_allocatorIcEEEENS1_10basic_jsonIcNS1_13sorted_policyES9_EEEENS8_ISE_EEJRSA_SD_EEPT_SI_RKT0_DpOT1_EUlDpOT_E_JSG_SD_RKSF_EENSt15__invoke_resultISH_JDpT0_EE4typeEOSH_DpOSW_(struct {...} && __fn) (/usr/include/c++/11/bits/invoke.h:96)
_ZSt12__apply_implIZSt39uninitialized_construct_using_allocatorIN8jsoncons9key_valueINSt7__cxx1112basic_stringIcSt11char_traitsIcENSt3pmr21polymorphic_allocatorIcEEEENS1_10basic_jsonIcNS1_13sorted_policyES9_EEEENS8_ISE_EEJRSA_SD_EEPT_SI_RKT0_DpOT1_EUlDpOT_E_St5tupleIJSG_OSD_RKSF_EEJLm0ELm1ELm2EEEDcOSH_OSJ_St16integer_sequenceImJXspT1_EEE(struct {...} && __f, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >&&, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > const&> && __t) (/usr/include/c++/11/tuple:1854)
_ZSt5applyIZSt39uninitialized_construct_using_allocatorIN8jsoncons9key_valueINSt7__cxx1112basic_stringIcSt11char_traitsIcENSt3pmr21polymorphic_allocatorIcEEEENS1_10basic_jsonIcNS1_13sorted_policyES9_EEEENS8_ISE_EEJRSA_SD_EEPT_SI_RKT0_DpOT1_EUlDpOT_E_St5tupleIJSG_OSD_RKSF_EEEDcOSH_OSJ_(struct {...} && __f, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >&&, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > const&> && __t) (/usr/include/c++/11/tuple:1865)
std::uninitialized_construct_using_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >(jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * __p, const std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > & __a) (/usr/include/c++/11/bits/uses_allocator_args.h:207)
std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > >::construct<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >(std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > * const this, jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * __p) (/usr/include/c++/11/memory_resource:319)
std::allocator_traits<std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > >::_S_construct<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >(std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > & __a, jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * __p) (/usr/include/c++/11/bits/alloc_traits.h:251)
std::allocator_traits<std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > >::construct<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >(std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > & __a, jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * __p) (/usr/include/c++/11/bits/alloc_traits.h:364)
std::vector<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > >::emplace_back<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >&, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >(std::vector<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > >, std::pmr::polymorphic_allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > > > * const this) (/usr/include/c++/11/bits/vector.tcc:115)
jsoncons::sorted_json_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >, std::vector>::uninitialized_init(jsoncons::sorted_json_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::pmr::polymorphic_allocator<char> >, jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >, std::vector> * const this, jsoncons::index_key_value<jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> > > * items, std::size_t count) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_object.hpp:563)
jsoncons::json_decoder<jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >, std::allocator<char> >::visit_end_object(jsoncons::json_decoder<jsoncons::basic_json<char, jsoncons::sorted_policy, std::pmr::polymorphic_allocator<char> >, std::allocator<char> > * const this) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_decoder.hpp:165)
jsoncons::basic_json_visitor<char>::end_object(jsoncons::basic_json_visitor<char> * const this, const jsoncons::ser_context & context, std::error_code & ec) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_visitor.hpp:265)
jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> >::end_object(jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> > * const this, jsoncons::basic_json_visitor<char> & visitor, std::error_code & ec) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_parser.hpp:361)
jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> >::parse_some_(jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> > * const this, jsoncons::basic_json_visitor<char> & visitor, std::error_code & ec) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_parser.hpp:782)
jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> >::parse_some(jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> > * const this, jsoncons::basic_json_visitor<char> & visitor, std::error_code & ec) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_parser.hpp:533)
jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> >::finish_parse(jsoncons::basic_json_parser<char, std::pmr::polymorphic_allocator<char> > * const this, jsoncons::basic_json_visitor<char> & visitor, std::error_code & ec) (/home/shahar/dragonfly/build-dbg/third_party/libs/jsoncons/include/jsoncons/json_parser.hpp:550)
dfly::JsonFromString(std::string_view input) (/home/shahar/dragonfly/src/core/json_object.cc:29)
[...]

From which I learn that polymorphic allocator is in fact used to call new, however it's not the right allocator. The reason it's not the right allocator comes down to the fact that sorted_json_object attempts to use std::pmr::polymorphic_allocator<jsoncons::key_value...> as its vector allocator, while the constructor for key_value expects a std::pmr::polymorphic_allocator<char>.

I'm still learning about allocators world, so I'm not sure what's the best solution (or even if I could do anything on our side to fix this), but perhaps you'd know?

Thanks again!

danielaparker commented 9 months ago

Yes, you're right, it appears that my understanding of the behaviour of std::pmr::polymorphic_allocator was incorrect. I'll look into this, but it's unlikely to be a quick fix.

chakaz commented 9 months ago

Thank you, I appreciate it!

danielaparker commented 9 months ago

@chakaz can you check branch main and see if the issue is resolved?

chakaz commented 9 months ago

Indeed! I can confirm that using main I do not see out of allocator behavior, at least not in my use case :) Thanks a lot!

chakaz commented 9 months ago

Thanks again :)