Skip to content

performance: changing Infra and Xds IR log values to JSONString#4263

Merged
Xunzhuo merged 1 commit intoenvoyproxy:mainfrom
shawnh2:fix-mem
Sep 19, 2024
Merged

performance: changing Infra and Xds IR log values to JSONString#4263
Xunzhuo merged 1 commit intoenvoyproxy:mainfrom
shawnh2:fix-mem

Conversation

@shawnh2
Copy link
Copy Markdown
Contributor

@shawnh2 shawnh2 commented Sep 17, 2024

What type of PR is this?

What this PR does / why we need it:

changing infra and xds IR log value from yaml to json

before:

image

after:

image

Which issue(s) this PR fixes:

fix #3698

before:

image

after:

image

AYCS, the highest mem cost has been decreased from 1370MB to 590MB.

Signed-off-by: shawnh2 <shawnhxh@outlook.com>
@shawnh2 shawnh2 requested a review from a team as a code owner September 17, 2024 05:24
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 65.65%. Comparing base (a8b0d2b) to head (474f571).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
internal/ir/infra.go 0.00% 3 Missing ⚠️
internal/ir/xds.go 0.00% 3 Missing ⚠️
internal/gatewayapi/runner/runner.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4263      +/-   ##
==========================================
+ Coverage   65.61%   65.65%   +0.03%     
==========================================
  Files         197      197              
  Lines       23551    23557       +6     
==========================================
+ Hits        15454    15466      +12     
+ Misses       6983     6981       -2     
+ Partials     1114     1110       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Sep 17, 2024

hey @shawnh2 thanks for improving the perf !
can you share a before & after of what the logs look like in the PR description

return string(y)
}

func (x Xds) JSONString() string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Struct Xds has methods on both value and pointer receivers. The Go Documentation does not recommend such usage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed, Xds has methods on both value and pointer receivers, this may be revisited later, not a primary concern in this PR.

return string(y)
}

func (i Infra) JSONString() string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Struct Infra has methods on both value and pointer receivers. Such usage is not recommended by the Go Documentation.

@shawnh2
Copy link
Copy Markdown
Contributor Author

shawnh2 commented Sep 18, 2024

hey @shawnh2 thanks for improving the perf ! can you share a before & after of what the logs look like in the PR description

updated

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.

Why the memory is not increase linearly when the HTTPRoutes scales up?

5 participants