Skip to content

util: Add test for MemBlockBuilder::release(), which was not working (discovered in #8779).#9360

Merged
jmarantz merged 3 commits intoenvoyproxy:masterfrom
jmarantz:memblock-cleanup
Dec 16, 2019
Merged

util: Add test for MemBlockBuilder::release(), which was not working (discovered in #8779).#9360
jmarantz merged 3 commits intoenvoyproxy:masterfrom
jmarantz:memblock-cleanup

Conversation

@jmarantz
Copy link
Contributor

@jmarantz jmarantz commented Dec 16, 2019

Description: Follow-up to #9306 which added a release() method, but after moving to an internal span representation it stopped working, and there was not a unit test for it. This adds the unit test and fixes the impl. I had promised follow-ups on 2 other issues raised by @mattklein123 in #9306 , but I'm leaning against one of them, and modified the other.

  • Reverting the write_span_ representation: I've grown enamored of it because it makes the implementation terser and easier to be confident in. We can re-visit if a perf bottleneck is measured.
  • Renaming populate() to resize() -- as explained in the comment for populate(), that method does not have resize semantics: old data is not preserved. However I renamed to setCapacity per discussion.

Risk Level: low
Testing: just the MemBlockBuilder test.
Docs Changes: n/a
Release Notes: n/a

…xy#8779).

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
mattklein123
mattklein123 previously approved these changes Dec 16, 2019
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM w/ one optional comment.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice!

@jmarantz jmarantz merged commit a23eff5 into envoyproxy:master Dec 16, 2019
@jmarantz jmarantz deleted the memblock-cleanup branch December 16, 2019 20:04
jmarantz added a commit to jmarantz/envoy that referenced this pull request Dec 16, 2019
Signed-off-by: Joshua Marantz <jmarantz@google.com>
jmarantz added a commit to jmarantz/envoy that referenced this pull request Dec 16, 2019
Signed-off-by: Joshua Marantz <jmarantz@google.com>
prakhag1 pushed a commit to prakhag1/envoy that referenced this pull request Jan 3, 2020
…(discovered in envoyproxy#8779). (envoyproxy#9360)

* Add test for release(), which was not working (discovered in envoyproxy#8779).

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Prakhar <prakhar_au@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants