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

fix(dht): fix over allocation for encrypted messages #4832

Merged
merged 3 commits into from
Oct 20, 2022

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Oct 20, 2022

Description

  • Fix over-allocation of message capacity for encrypted messages.
  • Minor optimisation using resize to improve 0 padding performance

Motivation and Context

reserve argument is additional bytes not total.
Fixes #4829

resize also allows the addition of padding bytes using raw pointer operations which are presumed to be much quicker than using rust iterators.

How Has This Been Tested?

Updated unit test to check for a correct capacity.

@sdbondi sdbondi changed the title fix(dht): fix over allocation for encryupted messages fix(dht): fix over allocation for encrypted messages Oct 20, 2022
@CjS77 CjS77 added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 20, 2022
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@jorgeantonio21
Copy link
Contributor

LGTM

@CjS77 CjS77 removed the P-acks_required Process - Requires more ACKs or utACKs label Oct 20, 2022
CjS77
CjS77 previously approved these changes Oct 20, 2022
Copy link
Collaborator

@CjS77 CjS77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CjS77 CjS77 added P-merge Process - Queued for merging and removed P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 20, 2022
@sdbondi sdbondi marked this pull request as draft October 20, 2022 13:32
@sdbondi sdbondi marked this pull request as ready for review October 20, 2022 13:34
@CjS77 CjS77 merged commit d29a64c into tari-project:development Oct 20, 2022
@sdbondi sdbondi deleted the dht-fix-over-alloc branch October 20, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-merge Process - Queued for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Message padding allocates buffer space unnecessarily
4 participants