Rapidjson: Storing values for later use

[kadst43]

I have perhaps a bit of an unusual use case... I want to load in one or more files that contain user-created configuration data for my application. I then want to parse the top layer or two of these files, pull out the values, and store them in my own data structure, for later use. I'd rather not have to keep the original document around, if only because I don't know how many files I'll be loading (it's the user's prerogative to decide that). So, my original design has been to load the file into a char*, parse it in situ, and then make deep copies of the values which I need to store for later use using the copy constructor.

The problem that I'm having seems to be that the allocator is owned by the document, so when I clean up the document it also deletes the allocator, which deletes all of those deep copies that I made. I tried using a separate allocator (not the one that belongs to the document), but that didn't appear to work (I think maybe it still uses the document's allocator for some things? I'm not sure.). I tried having a separate allocator, and then passing it into the document's constructor, but then the document takes ownership of it and cleans it up. I tried using a CRT allocator, but didn't have luck with that either.

So my question is, is there a clean way to do what I'm wanting to do? I'd love to use the memory pool allocator if I can, I'd be willing to use the CRT allocator if I have to...

I can share code if I need to, but hopefully the issue is clear from what I wrote? Since none of my code is right, I'm not sure how helpful it will be...

[miloyip]

You may keep a document for each deep copy.
Allocator should work too. Can you show some code to show how it does not work?

[kadst43]

Found the problem. It wasn't the allocator or deleting the copy, it was that I deleted the char* as well. So this code is bad:

` char* fileBuffer = /* allocate a char array on the heap, load the file into it */

MemoryPoolAllocator<> alloc;

Document* pDoc = new Document();

pDoc->ParseInsitu(fileBuffer);

// ... extract the values, make deep copies, store the copies

delete pDoc;

delete[] fileBuffer;   // <== THE PROBLEM!!!!!!`

That begs another question I wanted to ask, though... what is the right way to clean up that char* in this situation, assuming that it's not created on the stack (which it can't be, because I don't know the file size until runtime)? Do I just need to leak it? Keep a pointer to it and delete it when I'm done with the stored values? Or, alternately, should I just keep it around and then not bother to make the deep copies?

[pah]

Hmm, if making a deep copy with a from an in-situ result, it's indeed an inconvenient situation that you currently can't copy the "referenced" strings - still pointing into the in-situ buffer. I have now proposed PR #964 to give you control over that behavior for your use case.

With this, you can safely cleanup the fileBuffer after you copied the result somewhere else (using the new flag, of course).

[Llerd]

Just ran getting garbled strings after Insitu parsing myself. It looks like it isn't propagating the copyConstStrings option when parsing arrays and objects. Adding that to placement new on line 628, 629 and 642 fixed that for me.

[pah]

Duh, good catch! Submitted #969 to include your fix.

[Llerd]

I've been working with this approach for some time now, and while everything works, I'm not sure it's ideal. My scenario is the same as described above where I load a .json as a single data-chuck, pass it into Insitu parsing and the do processing on it before releasing both the document and the data-chunk. Where I need to retain the parts of the json for later use, I copy it with copyConstStrings. However, in my case I only need to do that for bits and pieces here and there, and that has led to hard-to-track-down bugs where I missed this flag.

I'm wondering if it would have been better to pass noConstStrings flag into the ParseInsitu function instead. I don't know if rapidjson internal format allows this or not, but this would tag the strings as non-const, while still referencing to the data-chunk. This way, the would be properly copied without the copyConstStrings option, making it less bug-prone and less chance of the user setting the copyConstString flag "just to be sure" (as I do now in some cases).

[pah]

I'm wondering if it would have been better to pass noConstStrings flag into the ParseInsitu function instead. I don't know if rapidjson internal format allows this or not, but this would tag the strings as non-const, while still referencing to the data-chunk.

The internal format would allow this, but not the current Handler concept, used by the parser to feed that values into the Document.

I agree that the current interface can be dangerous for your use case. On the other hand, extending the Handler by yet another set of string overloads to address this corner case feels a bit awkward to me. But maybe I just don't have a good idea, how such an interface could look like.

[Llerd]

Got it. My guess would be that the cleanest implementation would be to pass an enum with three states to the handler instead of the "bool copy" flag. But it adds complexity to solve a corner-case, so I defer to your understanding of the underlying systems.

Now, if the internal format allows this, an alternative for me would be to traverse the document directly after parsing and re-tag all strings to be non-const. Is this possible?

[pah]

Now, if the internal format allows this, an alternative for me would be to traverse the doc and re-tag all strings to be non-const. Is this possible?

Not from the outside. The corresponding parts of the implementation are private, see document.h:

 1885:         kConstStringFlag = kStringType | kStringFlag,
 1886:         kCopyStringFlag = kStringType | kStringFlag | kCopyFlag,
...
 1897:     struct Flag {

[Llerd]

Great. I'll try modifying that and see if that is the way to go for me. Thanks!