[cuebot/cuegui/pycue/rqd] Store PSS and MaxPSS #2112
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DiegoTavares
force-pushed
the
pss_rss_memory
branch
from
c303484 to
31e5cc4
Compare
DiegoTavares
requested review from
lithorus and
ramonfigueiredo
as code owners
ramonfigueiredo
changed the title
ramonfigueiredo
approved these changes
Jan 7, 2026
Collaborator
ramonfigueiredo
left a comment
ramonfigueiredo
left a comment
There was a problem hiding this comment.
LGTM with minor suggestions
Comment on lines
+19
to
+28
| ADD int_max_pss INT DEFAULT 0; | ||
|
|
||
| -- Add PSS fields to layer_resource table | ||
| ALTER TABLE layer_resource | ||
| ADD int_max_pss INT DEFAULT 0; | ||
|
|
||
| -- Add PSS fields to proc table | ||
| ALTER TABLE proc | ||
| ADD int_pss_max_used BIGINT DEFAULT 0, | ||
| ADD int_pss_used BIGINT DEFAULT 0; |
Collaborator
There was a problem hiding this comment.
Data Type Inconsistency
job_mem.int_max_pssandlayer_mem.int_max_pssuse INT (4 bytes, max ~2TB in KB)frame.int_pss_max_usedandproc.int_pss_max_useduse BIGINT (8 bytes)
For consistency and future-proofing, consider using BIGINT for all PSS fields. While 2TB is unlikely to be reached, using the same type across all tables improves maintainability.
| max_used_gpu_memory: (stats.max_used_gpu_memory / KIB) as i64, | ||
| used_gpu_memory: (stats.used_gpu_memory / KIB) as i64, | ||
| children, | ||
| ..Default::default() |
Collaborator
There was a problem hiding this comment.
Missing used_swap_memory in RunningFrameInfo
The RunningFrameInfo in clone_into_running_frame_info() uses ..Default::default() which means used_swap_memory (field 19) is not being set. This appears to be pre-existing, but worth noting if swap tracking is important.
| int64 max_gpu_memory = 11; | ||
| int64 used_gpu_memory = 12; | ||
| FrameStateDisplayOverride frame_state_display_override = 13; | ||
| } |
Collaborator
There was a problem hiding this comment.
UpdatedFrame Missing PSS Fields
- The
UpdatedFramemessage (used for incremental frame updates) doesn't include PSS fields (max_pss,used_pss). This means theFrameMonitorTreewon't show updated PSS values during incremental updates - only on full refreshes.
| byte[] children = new byte[100]; | ||
|
|
||
| procDao.updateProcMemoryUsage(frame, 100, 100, 1000, 1000, 0, 0, 0, children); | ||
| procDao.updateProcMemoryUsage(frame, 100, 100, 100, 100, 1000, 1000, 0, 0, 0, children); |
Collaborator
There was a problem hiding this comment.
Test coverage concern:
- The test change only adds one line for PSS verification. Consider adding more comprehensive tests for:
- PSS aggregation logic in
updateJobMemoryUsageandupdateLayerMemoryUsage - Edge cases (PSS > RSS, PSS = 0)
- PSS aggregation logic in
Comment on lines
88
to
+91
| /// Amount of memory used by all processes in this session | ||
| memory: u64, | ||
| rss: u64, | ||
| /// Amount of memory used by all processes in this session | ||
| pss: u64, |
Collaborator
There was a problem hiding this comment.
Documentation comment inconsistency.
/// Amount of memory used by all processes in this session
rss: u64,
/// Amount of memory used by all processes in this session
pss: u64,The pss comment is identical to rss. Should differentiate (as done in linux.rs:116-118):
/// Amount of memory used by all processes in this session calculated by rss
rss: u64,
/// Amount of memory used by all processes in this session calculated by pss
pss: u64,
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Update modules to properly store PSS (Proportional Set Size) alongside RSS (Resident Set Size) for measuring process memory usage on Linux systems.
Key Differences:
/proc/[pid]/smaps_rollup(Linux kernel 4.14+) provides pre-summed PSS valuesCurrent State:
Rqd contains a config property to collect memory as either RSS or PSS. Information is saved on Cuebot and displayed as Rss.
Proposed Solution:
Rqd collects and reports both RSS and PSS. Cuebot stores both in the database. pycue and cuegui display both.
Tasks: