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

Remove possible intermediate states of the http request conversion to AST #629

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pushpalanka
Copy link
Contributor

@Pushpalanka Pushpalanka commented Jan 2, 2025

Remove intermediate conversions between the HTTP request to AST input.

Motive: From the below profile capture it was noticed most of the CPU time is spent on these conversions. (It is from a slightly older version of OPA)

image

Pending new results..

Signed-off-by: Pushpalanka Jayawardhana <[email protected]>
@ashutosh-narkar
Copy link
Member

Does this PR achieve the objective of this PR?

@Pushpalanka
Copy link
Contributor Author

Does this PR achieve the objective of this PR?

Thanks for the reference @ashutosh-narkar.

After having a further look we are now trying the possibility of getting rid of all the intermediate states than just JSON conversions and translating http request directly to the AST.Values. (mainly this section) Will update the PR accordingly. Please do share if you see in pitfalls with the approach.

@Pushpalanka Pushpalanka changed the title Remove intermediate JSON marshal and unmarshal Remove possible intermediate states of the http request conversion to AST Jan 9, 2025
@ashutosh-narkar
Copy link
Member

That seems reasonable. If you have any benchmarks that show the performance improvement that would be great to see.

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