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

Reduce Route memory utilization by avoiding RuntimeData instances when not needed #24327

Merged
merged 5 commits into from
Dec 8, 2022

Conversation

yurykats
Copy link
Contributor

@yurykats yurykats commented Dec 2, 2022

Reduce Route memory utilization by avoiding RuntimeData instances when not needed

Currently an instance of RuntimeData (wrapped in optional) is created in each route, even if no runtime fractions are configured. This PR switches the instance to unique_ptr and only creates RuntimeData objects when actually configured.

This is a step towards more memory efficient config data structures. Issue #24154.

Signed-off-by: Yury Kats ykats@google.com

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

…n not needed

Currently an instance of RuntimeData is created in each route, even if no runtime fractions are configured. This PR switches the instance to unique_ptr and only creates RuntimeData objects when actually configured.

This is a step towards more memory efficient config data structures. Issue envoyproxy#24154.

Signed-off-by: Yury Kats <ykats@google.com>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #24327 was opened by yurykats.

see: more, trace.

Signed-off-by: Yury Kats <ykats@google.com>
@yurykats yurykats marked this pull request as ready for review December 2, 2022 17:49
@yurykats
Copy link
Contributor Author

yurykats commented Dec 2, 2022

/assign @adisuissa @zuercher

Copy link
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@yurykats
Copy link
Contributor Author

yurykats commented Dec 6, 2022

@zuercher please take a look, thanks!

@adisuissa adisuissa enabled auto-merge (squash) December 7, 2022 14:53
@yurykats
Copy link
Contributor Author

yurykats commented Dec 7, 2022

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Check envoy-presubmit didn't fail.

🐱

Caused by: a #24327 (comment) was created by @yurykats.

see: more, trace.

@phlax
Copy link
Member

phlax commented Dec 7, 2022

@yurykats i kicked the pipeline manually - not sure if /retest would have worked - we are making changes to some of the CI which are a bit disruptive

@yurykats
Copy link
Contributor Author

yurykats commented Dec 7, 2022

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24327 (comment) was created by @yurykats.

see: more, trace.

@adisuissa adisuissa merged commit 215a86a into envoyproxy:main Dec 8, 2022
@yurykats yurykats deleted the config_memory_5 branch December 8, 2022 01:08
jpsim added a commit that referenced this pull request Dec 8, 2022
…-cpp-to-latest-version

* origin/main: (23 commits)
  Reduce Route memory utilization by avoiding RuntimeData instances when not needed (#24327)
  build: fix compile error for mac (#24429)
  postgres: support for upstream SSL (#23990)
  iOS: split `EnvoyEngine.h` into multiple header files (#24397)
  mobile: check for pending exceptions after JNI call (#24361)
  Remove uneccessary `this->` from mobile engine builder (#24389)
  Add setRequestDecoder to ResponseEncoder interface (#24368)
  downstream: refactoring code to remove listener hard deps (#24394)
  lb api: moving load balancing policy specific configuration to extension configuration (#23967)
  ci: Skip docker/examples verification for docs or mobile only changes (#24417)
  ci: run mobile GitHub Actions on every PR (#24407)
  mobile: remove `bump_lyft_support_rotation.sh` script (#24404)
  Add file size to DirectoryEntry (#24176)
  bazel: update to 6.0.0rc4 (#24235)
  bazel: update rules_rust (#24409)
  Ecds config dump recommit (#24384)
  bazel: add another config_setting incompatible flag (#24270)
  listeners: moving listeners to extension directory (#24248)
  mobile: build Swift with whole module optimization (#24396)
  ci: update `actions/setup-java` from v1 to v3.8 (#24393)
  ...

Signed-off-by: JP Simard <jp@jpsim.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.

None yet

4 participants