fix: compute_memory_size() added #5
Conversation
|
As far as I could find, the is no way to compute the requested memory usage, neither the real memory usage (i.e. mi_good_size) as the library doesn't have control over this. What it does support is polymorphic allocators and we're already using them to get the total memory usage of all json objects. A simpler solution in terms of code, but with higher overhead, would be "copying" the json object with a custom memory resource and counting the memory usage from within. Should be just a few lines of code inside Dragonfly... Though we'll end up copying the object |
|
If I understand correctly, the code traverses the object tree and computes
recursively memory usage.
Is there an interface that traverses with a generic visitor? In this case ,
we won't need a mimalloc specific code here as we could pass the visitor
inside dragonfly code
Roman Gershman
CTO
---
*www.dragonflydb.io <http://www.dragonflydb.io>*
…On Wed, Oct 8, 2025, 12:42 PM Vladislav ***@***.***> wrote:
*dranikpg* left a comment (dragonflydb/jsoncons#5)
<#5 (comment)>
As far as I could find, the is no way to compute the requested memory
usage, neither the real memory usage (i.e. mi_good_size) as the library
doesn't have control over this. What it does support is polymorphic
allocators and we're already using them to get the total memory usage of
all json objects.
A simpler solution in terms of code, but with higher overhead, would be
"copying" the json object with a custom memory resource and counting the
memory usage from withing. Should be just a few lines of code inside
Dragonfly... Though we'll end up copying the object
—
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4BFCA6HH3RTXRCRRQGFJ33WTTBLAVCNFSM6AAAAACIK4N3Z2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOBQHEYTIMRTGE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
@romange |
include/jsoncons/basic_json.hpp
Outdated
| // Callback type for getting usable size of allocated memory | ||
| using memory_size_callback = std::function<std::size_t(const void*)>; | ||
|
|
||
| // Recursive implementation of compute_memory_size with callback |
There was a problem hiding this comment.
Won't this blow the stack for large objects ? (fibers do not have a lot of stack memory)
There was a problem hiding this comment.
Fixed. No recursion any longer.
|
Do these tests fail without your changes as well? |
Yes, I didn't change any code in the library; I added only one method. My initial version was under the define macro, and tests failed, but the code should be the same as before. My guess is the CI should be set specifically. |
Co-authored-by: Roman Gershman <romange@gmail.com> Signed-off-by: Volodymyr Yavdoshenko <v.yavdoshenko@gmail.com>
include/jsoncons/basic_json.hpp
Outdated
| // Optimization: only add elements that have dynamic memory | ||
| for (const auto& elem : arr) | ||
| { | ||
| auto kind = elem.storage_kind(); |
There was a problem hiding this comment.
why not use if (elem.capacity() > 0) here?
include/jsoncons/basic_json.hpp
Outdated
| const auto& value = member.value(); | ||
| auto kind = value.storage_kind(); | ||
| // Skip inline storage types (primitives, short strings, empty objects) | ||
| if (kind != json_storage_kind::null && |
There was a problem hiding this comment.
not only it's more verbose than calling capacity(), you also need to duplicate this.
|
@romange |
4 participants
This fix is to be used for dragonflydb/dragonfly#5480