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

buffer-management #26

Open
jamesdbrock opened this issue Oct 7, 2019 · 3 comments
Open

buffer-management #26

jamesdbrock opened this issue Oct 7, 2019 · 3 comments

Comments

@jamesdbrock
Copy link
Owner

Split out the buffer-management features of message_reader and message_writer into separate classes?

The issue I guess comes down to

for (; reader.is_complete(); reader = reader.next_message_reader()) {

and

hffix::message_writer new_order(logon.message_end(), buffer + sizeof(buffer));

being kind of weird.

@jamesdbrock
Copy link
Owner Author

(These lines are from the example programs writer01.cpp and reader01.cpp.)

On reflection, this is fine:

for (; reader.is_complete(); reader = reader.next_message_reader()) {

But maybe this

hffix::message_writer new_order(logon.message_end(), buffer + sizeof(buffer));

should be more like:

hffix::message_writer new_order = logon.next_message_writer();

Though the current system is a pretty simple API and if we are going to improve it, we want to make sure that the improvements are even simpler.

next_message_writer() method should explain that it returns a new message_writer constructed on the same buffer, but beginning where the current message ended.

We cannot, in general, ensure that when a message_writer is constructed that it will have enough space to write the message, so we shouldn't try to do that.

The problem with this whole thing is that is has the effect of hiding the buffer arithmetic instead of making the buffer arithmetic explicit and transparent. If we append messages to the buffer with next_message_writer(), then when it's time to do I/O we go back to explicit buffer arithmetic with

std::cout.write(buffer, new_order.message_end() - buffer);

@dingobye
Copy link

dingobye commented Oct 8, 2019

I think the current design is alright.

It makes sense for users to explicitly mange the buffer arithmetics, since message_writer is designed not to own any buffer, but only works on a given chunk of contiguous memory (with begin_ and end_). It is the user's responsibility to organise the offsets of one buffer shared by multiple message_writer instances, who are lack of a global view as individuals.

@jamesdbrock
Copy link
Owner Author

I suspect that reader = reader.next_message_reader is causing #27 . What we really need is a whole message_reader_buffer abstraction with iterators which dereference to message_readers. That would be a massive breaking API change though.

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

No branches or pull requests

2 participants