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

Several tips #17

Closed
nxrighthere opened this issue Feb 11, 2019 · 9 comments

Comments

@nxrighthere
Copy link

commented Feb 11, 2019

(A note for the end-user: don't use this bad abstraction of the ENet in any of your projects, especially if you are creating a non-hobby stuff)

  1. Don't allocate buffers dynamically on the heap here and here. If arrays are not moving across threads, then allocate them once like this and reuse all the time. You need to expose API to provide the actual length of the payload since you need to allocate large buffers. If arrays are moving across threads, use ArrayPool from NetStack or System.Buffers from NuGet, the implementation is almost the same.

  2. You are currently dispatching one event per frame which will lead to such issues. Read my last comment there.

  3. You should separate channels and delivery types as it was designed originally, right now you force head-of-line blocking which is counter-productive when you use multiplexed UDP transport.

  4. You should include .meta files for native libraries and explicitly assign each to appropriate platforms and CPU architectures, especially for Android.

@SoftwareGuy

This comment has been minimized.

Copy link
Owner

commented Feb 12, 2019

I'm not sure about the first point. Are you saying it's better to make a large memory buffer and copy the contents of packets into that rather than creating a byte array and then letting it get cleaned up by garbage collection?

Point number 2 is scheduled for v1.2 of the transport. Could you please elaborate further on point number 3?

@nxrighthere

This comment has been minimized.

Copy link
Author

commented Feb 12, 2019

Are you saying it's better to make a large memory buffer and copy the contents of packets into that rather than creating a byte array and then letting it get cleaned up by garbage collection?

Yes, you allocate it only once and then reuse. A perfect solution is to eliminate memory copying from native to managed memory and use Span<T> for direct access, but you are behind the 3rd-party wall.

Could you please elaborate further on point number 3?

You have two channels right now, one for unreliable packets and one for reliable, while ENet offers 255 for logic separation to fight latency.

@nxrighthere

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

One more thing. You are currently dispatching one event per frame which will lead to such issues. Read my last comment there.

@SoftwareGuy

This comment has been minimized.

Copy link
Owner

commented Feb 15, 2019

I'm working on a new packet processing engine based on that post's example which, if it works out, might be more efficient/effective. Still not 100% sure how to eliminate stack allocations as much as possible since some of that is out of my league.

@nxrighthere

This comment has been minimized.

Copy link
Author

commented Feb 15, 2019

You removing allocations on the heap, not on the stack. Managed arrays are always allocated on the heap until you use stackalloc. Array pooling will solve this, but you need to provide the actual length of the payload so the user may slice array using segment or Span.

@SoftwareGuy

This comment has been minimized.

Copy link
Owner

commented Feb 15, 2019

Unfortunately, this is the sort of thing that is way out of my league, and I'd appreciate someone to submit a PR that can fill in the gap. I'd need a primer to get me started, or at least something that is easy enough to understand and learn from examples. NetStack might be an interesting thing for me to look into.

@SoftwareGuy

This comment has been minimized.

Copy link
Owner

commented Feb 15, 2019

Hold up, netstack is your own library?!

@nxrighthere

This comment has been minimized.

Copy link
Author

commented Feb 15, 2019

Yes, and array pooling example is available there.

@SoftwareGuy

This comment has been minimized.

Copy link
Owner

commented Sep 21, 2019

Going to close this, please feel free to reopen if there's any additional feedback. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.