Jump to content
  • Advertisement
Sign in to follow this  
thedustbustr

trying to get some simple network buffering code right

This topic is 3746 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

The data sent over the socket is an infinite series of nul-terminated strings. Each string found must be passed to the UnpackFrame function - this function interface is (char* str, unsigned size). While I do control this interface, I'm going to be calling it from Python ctypes (and potentially another less featured scripting language), so it is constrained to primitives. Is this correct? For now I am verifying DLL functionality with C++. I was hoping to use boost::circular_buffer for my read buffer but I don't think that will work with boost::asio. My workaround is some vector math and an unnecessary copy which I want to eliminate. I'm pretty sure its wrong right now. Please help me get rid of nasty data copy hack at the end, and also any other critiques you have. It is my first day using boost::asio.
try 
{
	boost::asio::io_service io_service;
	tcp::acceptor acceptor(io_service, tcp::endpoint(tcp::v4(), 65123));

	std::vector<char> buf(1024);
	for (;;)
	{
		tcp::socket socket(io_service);
		acceptor.accept(socket);

		boost::system::error_code error;
		while (error != boost::asio::error::eof)
		{
			size_t len = socket.read_some(boost::asio::buffer(buf), error);
			if (error == boost::asio::error::eof) ; //clean close
			else if (error) throw boost::system::system_error(error);
			//std::cout.write(buf.data(), len);
				
			std::vector<char>::iterator frame_seperator;
			frame_seperator = std::find(buf.begin(), buf.end(), '\0');

			if (*frame_seperator == '\0')
			{
				std::string InBuffer(buf.begin(), frame_seperator);
				char* rawstr = InBuffer.c_str();

				UnpackFrame(rawstr, InBuffer.size());
				//...more processing...

				//get the processed data out of my vector, and get the 
				//remaining data in the buffer at the front of the vector
				std::vector<char>::iterator old_end = buf.end();
				std::copy(++frame_seperator, old_end, buf.begin();
				buf.erase(old_end, buf.end());
			}
		}
	}
}
catch (std::exception& e)
{
	std::cerr << e.what() << std::endl;
}


[Edited by - thedustbustr on June 17, 2008 8:41:13 AM]

Share this post


Link to post
Share on other sites
Advertisement
std::vector<char>::iterator frame_seperator;
frame_seperator = std::find(buf.begin(), buf.end(), '\0');




keep the frame_seperator pointer maintained outside the loop so that when asio hasnt read enough data you dont re-begin the find() from the start of the buffer. how much benefit you get of course depends on the size of the data asio throws back at you (indeterminate i think given that tcp is stream orientated).

note that the asio async_read functions support a predicate delimiter on the search which would relieve you of having to write this yourself - if you dont really need the non-blocking behavoir.

std::string InBuffer(buf.begin(), frame_seperator);
char* rawstr = InBuffer.c_str();
UnpackFrame(rawstr, InBuffer.size());




i would do it like this to avoid the string constructor and the string having to do a copy of the data

UnpackFrame( &buf[0], std::distance( buf.begin(), frame_seperator)





//get the processed data out of my vector, and get the 
//remaining data in the buffer at the front of the vector
std::vector<char>::iterator old_end = buf.end();
std::copy(++frame_seperator, old_end, buf.begin();
buf.erase(old_end, buf.end());




I am not sure about this. from your description of the problem a std::deque would be perfect - fast popping from both the front and back but unfortunately stl doesnt guarantee that the data is contiguous and so it would stuff up the UnpackFrame interface where you need an array pointer - although i have used deques like this without problems in past - its just not guaranteed.

edit - anything that gives you effecient popping from front and pushing to the back (eg liked list of small vectors) is not going to afford the contiguous array you want - so you might not have an option here with avoiding a copy somewhere.

I was using the asio streambufs rather than vectors for the clear istream /ostream operators but have moved to boost::iostream for more flexibility. my spec means i am more concerned with being able to monitor network status than focusing on raw throughput and performance and so i take a copy hit in some places (it allows me to intrusively monitor whats going on for extended logging).

there are some other people on the boards with more asio experience who might offer better advice.

[Edited by - chairthrower on June 16, 2008 7:31:17 PM]

Share this post


Link to post
Share on other sites
nice thoughtful post.

std::deque would also stuff up
size_t len = socket.read_some(boost::asio::buffer(buf), error);
, wouldn't it? The asio::buffer docs are hard to understand but I don't think they can take anything except b::array, std::vector, and char[].

I think as far as using a vector,
buf.erase(buf.begin(), frame_seperator);
frame_seperator = buf.begin();

removes the explicit copy. but there is an implicit copy here.

[Edited by - thedustbustr on June 17, 2008 8:26:23 AM]

Share this post


Link to post
Share on other sites
Quote:
std::deque would also stuff up

size_t len = socket.read_some(boost::asio::buffer(buf), error);

, wouldn't it? The asio::buffer docs are hard to understand but I don't think they can take anything except b::array, std::vector, and char[].


I was going to say it should be ok and that internally asio would be able to take any sequence container type that supported the right iterator interface - but from your doc it looks like i am wrong. I would check the source of a newly extracted boost svn or alternatively the seperately maintained non boost asio and see whether it might have changed - quite a few things were added recently in boost asio (< month)). It would seem a generally useful thing although in an async context having the io service merely hold onto an iterator facade between callback setup and callback seems a bit funny - so maybe this is the reason its not implemented (hope this made sense).

Quote:
buf.erase(buf.begin(), frame_seperator);

frame_seperator = buf.begin();


removes the explicit copy. but there is an implicit copy here.


yep there's a copy there - i do understand the desire to get it sort of perfected, but i would run with what you have working. if you're anything like me then new requirements and unforseen problems mean i am always rearranging these sorts of low level details until things finally stabalise. also i am sure you realise the copy cost is really not that expensive so perhaps taking the "profile first and only then optim..." kind of approach would be a decent rationalisation.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!