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

Allow option of using Memory mapped file(s) instead of direct buffers #121

Open
ivansenic opened this issue Sep 14, 2016 · 3 comments
Open

Comments

@ivansenic
Copy link

Hi,

We are using kryonet for some time now in our project. However, we could not afford to have limited byte buffers for read/write in TCP Connections, as we are sending objects that follow the composite pattern, so object sizes are not fixed and predicable in fact. So we tweaked a bit the code and tried to added ability to overcome this, but we just figured out that we did it wrong in a way and that client server communication is a bottleneck for us, especially when there are two or more clients connected.

So I am trying once again to figure out how to improve our own mistakes and one idea came to my mind and that's: Why not allowing users to initialize the client and/or server with mapped byte buffers pointing to the memory mapped files, instead of the direct buffers?

The benefits would be following:

  • I can select how big are the mapped files and this is really flexible ( I could easily select 4Gb for example). With direct memory this is not an option.
  • Write and read operations should be performant enough (check this blog from Martin - http://mechanical-sympathy.blogspot.rs/2011/12/java-sequential-io-performance.html)
  • The implementation does not to change at all, as it's already working with the byte buffer API, so basically we would need only new constructors for switching to mapped byte buffers..

What do you think, did I not consider something here? If you think this is valid option, I have nothing against in providing the pull request, I would just need some agreement on how to implement it.

@crykn
Copy link

crykn commented Jul 17, 2017

Sounds like a good change! Are you still interested in implementing it or have done it already?

@ivansenic
Copy link
Author

Hi @crykn,

Unfortunately in our project we decided to go away from kryonet. Thus I can not provide you any support here as simply I don't have for it. If this changes in future I will let you know. Sorry but I posted this almost a year ago 😄

@crykn
Copy link

crykn commented Jul 17, 2017

No problem, it was worth a try asking ;)

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

No branches or pull requests

2 participants