Skip to content

Conversation

@Jackie-Jiang
Copy link
Contributor

  • Fix the setup of ThreadResourceUsageProvider so that enabling/disabling CPU/memory measurement is idempotent
  • Use MXBean.getCurrentThreadAllocatedBytes() to read current thread allocated bytes when available. It is about 10% more efficient than MXBean.getThreadAllocatedBytes(long threadId)
  • Enhance logging

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the ThreadResourceUsageProvider class to make CPU and memory measurement enable/disable operations idempotent and improves performance by using getCurrentThreadAllocatedBytes() when available (approximately 10% more efficient than the previous approach). The changes include restructured initialization logic, enhanced logging, and updated benchmarks.

Changes:

  • Refactored the static initialization block to properly detect and initialize reflection methods for thread resource measurement
  • Modified setThreadCpuTimeMeasurementEnabled() and setThreadMemoryMeasurementEnabled() to be idempotent with improved logging
  • Updated getCurrentThreadAllocatedBytes() to prefer getCurrentThreadAllocatedBytes() over getThreadAllocatedBytes(threadId) for better performance
  • Simplified benchmark class and added new tests for enable/disable toggling

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
ThreadResourceUsageProvider.java Core refactoring of initialization, enable/disable logic, and memory reading methods with enhanced logging
BenchmarkThreadResourceUsageProvider.java Simplified benchmark structure with direct method calls instead of snapshot-based approach
TestThreadMXBean.java Added new tests to verify idempotent behavior of enable/disable operations

@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 41.48936% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.22%. Comparing base (0170477) to head (ae1e1f8).
⚠️ Report is 22 commits behind head on master.

Files with missing lines Patch % Lines
...ot/spi/accounting/ThreadResourceUsageProvider.java 41.48% 45 Missing and 10 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17541      +/-   ##
============================================
- Coverage     63.24%   63.22%   -0.03%     
  Complexity     1476     1476              
============================================
  Files          3167     3170       +3     
  Lines        189250   189538     +288     
  Branches      28962    29007      +45     
============================================
+ Hits         119691   119833     +142     
- Misses        60273    60401     +128     
- Partials       9286     9304      +18     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.19% <41.48%> (-0.02%) ⬇️
java-21 63.16% <41.48%> (-0.06%) ⬇️
temurin 63.22% <41.48%> (-0.03%) ⬇️
unittests 63.22% <41.48%> (-0.03%) ⬇️
unittests1 55.54% <39.36%> (-0.03%) ⬇️
unittests2 34.05% <24.46%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang Jackie-Jiang merged commit b8bf104 into apache:master Jan 21, 2026
18 checks passed
@Jackie-Jiang Jackie-Jiang deleted the resource_usage_provider branch January 21, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants