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 OPA filter dependency.base calculation in optional body parsing #3210

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

Pushpalanka
Copy link
Collaborator

@Pushpalanka Pushpalanka commented Aug 27, 2024

Due to a detected performance concern for CPU as seen below,

image

Memory profile

alloc_objects profile
image

Load test results

Using a request body as below with a simple policy to check email domain.
{
      "email": "loadtest@zalando.de",
      "user": {
        "id": "12345",
        "name": {
          "first_name": "John",
          "last_name": "Doe"},
        "contact": {
          "phone_number": "+49-123-456-7890",
          "address": {
            "street": "Main Street 123",
            "city": "Berlin",
            "zip_code": "10115",
            "country": "Germany"}}},
      "account": {
        "created_at": "2023-08-28T10:15:30Z",
        "status": "active",
        "preferences": {
          "language": "en",
          "currency": "EUR"},
        "subscriptions": {
          "newsletter": true,
          "alerts": {
            "email": true,
            "sms": false}}},
      "meta": {
        "last_login": "2023-08-28T10:20:00Z",
        "login_count": 42,
        "roles": [
          "user",
          "tester"],
        "tags": [
          "load-test",
          "test-user"]}
    }

Before this improvement
23 instances for 1000 rps
max GC pause : 500 µs (high number of small objects generated by the dependency.base functions seems to have contributed here)
max heap objects : 3.17M
max heap usage: 557 MB

After this improvement
11 instances for 1000 rps (50% reduced of CPU power)
max GC pause: 269 µs.
max heap objects : 1.28M (50% reduced.)
max heap usage: 236 MB

Bench-mark test results

Before
BenchmarkAuthorizeRequest/authorize-request-with-body-8 10002 1068249 ns/op 8432573 B/op 806 allocs/op

After
BenchmarkAuthorizeRequest/authorize-request-with-body-8 13461 587883 ns/op 8418624 B/op 566 allocs/op

@Pushpalanka Pushpalanka added enhancement optimization major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs performance labels Aug 27, 2024
@szuecs
Copy link
Member

szuecs commented Aug 30, 2024

@Pushpalanka please also --sign-off the commit (DCO).

@Pushpalanka Pushpalanka force-pushed the dependencyBase branch 3 times, most recently from b832ec3 to 88f10ff Compare August 30, 2024 16:12
@Pushpalanka Pushpalanka marked this pull request as ready for review August 30, 2024 16:33
Signed-off-by: Pushpalanka Jayawardhana <pushpalanka.jayawardhana@zalando.de>
@mjungsbluth
Copy link
Collaborator

👍

@MustafaSaber
Copy link
Member

maybe the image for the CPU profile after would be great also to visualise the difference

@MustafaSaber
Copy link
Member

👍

@MustafaSaber MustafaSaber merged commit 08aba89 into master Sep 2, 2024
14 checks passed
@MustafaSaber MustafaSaber deleted the dependencyBase branch September 2, 2024 08:00
@Pushpalanka
Copy link
Collaborator Author

Pushpalanka commented Sep 2, 2024

maybe the image for the CPU profile after would be great also to visualise the difference

Here we go,

image

memory profile, (at 1000 rps ~32MB for body parsing.)
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs optimization performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants