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

Added a gather() function to gather std::map into the root process; #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hguo
Copy link

@hguo hguo commented Nov 13, 2019

added StringBuffer as an alternative to MemoryBuffer, in order to
ease the serialization to std::string; serializeToString() is used
in the new gather() function.

added StringBuffer as an alternative to MemoryBuffer, in order to
ease the serialization to std::string; serializeToString() is used
in the new gather() function.
@mrzv
Copy link
Member

mrzv commented Nov 13, 2019

@hguo We are doing all merge requests on Kitware's GitLab. Can you move this there?

@mrzv
Copy link
Member

mrzv commented Nov 13, 2019

@hguo Also, when you do, can you give some context about what you are trying to accomplish?

@hguo
Copy link
Author

hguo commented Nov 13, 2019

I am not trying to accomplish any specific goal in this PR. I wrote and use these utility functions for my own ease in my multiple codes including FTK and vortexfinder, and I think it might be a good idea to make them official in DIY.

@mrzv
Copy link
Member

mrzv commented Nov 13, 2019

Let's move this to GitLab, but I still want to understand what this buys you. I.e., what does this do that regular MemoryBuffer doesn't?

@hguo
Copy link
Author

hguo commented Nov 13, 2019

MemoryBuffer and StringBuffer essentially do the same thing. The reason that I prefer strings is because of legacy codes:

  1. I have been converting my old protobuf based serialization code to DIY's serialization. Protobuf has serializeToString and parseFromString functions, so it makes it easier for me to keep using std::string instead of std::vector.

  2. I use LevelDB/RocksDB for persistent key-value storage of serialized data. The put() and get() functions in these embedded DB take std::string. I have to routinely convert std::vector to std::string in my code if I use MemoryBuffer.

I don't have a GitLab account yet but will do after I got one.

@mrzv
Copy link
Member

mrzv commented Nov 13, 2019

I see. Would it make sense instead to add a way to convert MemoryBuffer to a string and vice versa to initialize it from a string? I guess that would incur a copy overhead. I'm a little nervous about the code duplication between MemoryBuffer and StringBuffer.

@hguo
Copy link
Author

hguo commented Nov 13, 2019

That should work; the drawback is introducing extra memcpy.

Is it possible to specify the "backend" container for MemoryBuffer by using a template? The container can be std::vector by default.

template <typename Container=std::vector<char>>
struct MemoryBuffer : public BinaryBuffer 
{
 ...
};

@mrzv
Copy link
Member

mrzv commented Nov 13, 2019

Oh, that's an interesting idea. Let me think about it; there are a couple of thorny technical issues, but that would be a very good solution if we can make it work.

@hguo
Copy link
Author

hguo commented Nov 13, 2019

Maybe we can use typedef:

template <typename Container>
struct GenericMemoryBuffer : public BinaryBuffer 
{
  Container buf;
};

typedef GenericMemoryBuffer<std::vector> MemoryBuffer;
typedef GenericMemoryBuffer<std::string> StringBuffer;

@mrzv
Copy link
Member

mrzv commented Nov 13, 2019

Right, I was thinking the same. I'm actually looking into what happens if we just use std::string as a backend. It seems like with small changes most things work. But that may be too drastic of a change.

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

Successfully merging this pull request may close these issues.

None yet

2 participants