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

NNG Protocol Level Memory Enhancements #86

Merged
merged 1 commit into from
Feb 27, 2025
Merged

NNG Protocol Level Memory Enhancements #86

merged 1 commit into from
Feb 27, 2025

Conversation

shikokuchuo
Copy link
Owner

The mirai v2.0 architectural enhancement already improves memory usage, as we moved from:

host <-- `req/rep`--> dispatcher <-- `req/rep` --> daemon

to:

host <-- `req/rep`--> dispatcher <-- `poly` --> daemon

The poly protocol does not retain messages longer than necessary and is efficient in that regard.

This PR adds memory optimization for the remaining req/rep leg:

  • The requestor on host still retains a copy of the message until the reply is received. This is even the case when we've turned off NNG's automatic protocol-level retry mechanism.
  • The NNG source is patched to free the request message early as soon as the initial send is complete (as it won't be used again).
  • Substantially lower memory usage on host if many mirai requests are outstanding (for example in a map operation).

The NNG author has previously agreed that this would be a sensible amendment to the protocol, so if everything tests out here, the patch will be proposed to upstream.

@shikokuchuo shikokuchuo added the enhancement New feature or request label Feb 26, 2025
@shikokuchuo shikokuchuo added this to the v1.5.2 milestone Feb 26, 2025
@shikokuchuo
Copy link
Owner Author

@wlandau This is an important one for your use in targets. Are you fine to do some testing based on this branch / PR?

If it's easier for you to grab R-universe builds, I'm ok to go ahead and merge this first.

@wlandau
Copy link

wlandau commented Feb 27, 2025

All the tests in https://github.com/wlandau/crew/tree/main/tests/testthat and https://github.com/wlandau/crew/tree/main/tests/local pass on Mac OS and RHEL9 using shikokuchuo/mirai@640468b and e38f178.

@shikokuchuo
Copy link
Owner Author

Proposed to upstream in nanomsg/nng#2107. Merging this PR.

@shikokuchuo shikokuchuo merged commit 3e06b6b into main Feb 27, 2025
10 checks passed
@shikokuchuo shikokuchuo deleted the nng branch February 27, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants