Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
chakaz opened this issue Jan 30, 2024 · 7 comments · Fixed by microsoft/vcpkg#36591
Closed

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

chakaz opened this issue Jan 30, 2024 · 7 comments · Fixed by microsoft/vcpkg#36591

Comments

@chakaz
Copy link

chakaz commented Jan 30, 2024

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?

@chakaz chakaz added the Bug label Jan 30, 2024
@danielaparker
Copy link
Owner

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
Copy link
Author

chakaz commented Jan 31, 2024

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
Copy link
Owner

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
Copy link
Author

chakaz commented Feb 1, 2024

Thank you, I appreciate it!

@danielaparker
Copy link
Owner

danielaparker commented Feb 2, 2024

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

@chakaz
Copy link
Author

chakaz commented Feb 2, 2024

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
Copy link
Author

chakaz commented Feb 5, 2024

Thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants