Skip to content

ci: Response Cache memory growth test - #8363

Merged
pskiran1 merged 4 commits into
mainfrom
spolisetty_fix_cache_mem_leak
Sep 3, 2025
Merged

ci: Response Cache memory growth test#8363
pskiran1 merged 4 commits into
mainfrom
spolisetty_fix_cache_mem_leak

Conversation

@pskiran1

@pskiran1 pskiran1 commented Aug 25, 2025

Copy link
Copy Markdown
Member

What does the PR do?

  • Added a memory growth test for when the response cache is enabled.
  • The script generate_random_data.py creates random data to trigger cache misses, insertions, and evictions.
  • The new test case checks if memory growth is within normal limits.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs: triton-inference-server/core#449

Where should the reviewer start?

Test plan:

  • CI Pipeline ID: 33813824

Caveats:

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

Test script output - Without memory leak fix (Memory grows over time):

image

Test script output - With memory leak fix (Memory is stable):

image

@pskiran1 pskiran1 added the PR: ci Changes to our CI configuration files and scripts label Aug 25, 2025
@pskiran1
pskiran1 marked this pull request as ready for review August 25, 2025 16:51
set +e

# Log test duration and the graph for memory growth
MAX_ALLOWED_ALLOC=2 # MB

@pskiran1 pskiran1 Aug 25, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this threshold value based on observations from example runs. Please let me know if you have any recommendations for configuring it.
Thank you.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the averge memory growth before your fix?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It exceeds 3MB on running this test. For reference, I attached a screenshot showing memory growth before the fix.
If the fix is included, it is less than 0.5 MB

@yinggeh yinggeh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work

@pskiran1
pskiran1 merged commit 3e52154 into main Sep 3, 2025
3 checks passed
@pskiran1
pskiran1 deleted the spolisetty_fix_cache_mem_leak branch September 3, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: ci Changes to our CI configuration files and scripts

Development

Successfully merging this pull request may close these issues.

2 participants