Skip to content

Refactor controller_track and system_trace - #981

Merged
amokiche-amd merged 15 commits into
mainfrom
amokichev/controller-refactoring-one-more-time
Aug 31, 2026
Merged

Refactor controller_track and system_trace#981
amokiche-amd merged 15 commits into
mainfrom
amokichev/controller-refactoring-one-more-time

Conversation

@amokiche-amd

@amokiche-amd amokiche-amd commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Motivation

Refactor controller Track and System_trace files to replace of usage default getters to direct getters and setters.
Refactor giant functions to increase code readability.
Small minor refactors.

Technical Details

Test Plan

Test Result

Submission Checklist

Comment thread src/model/src/datamodel/rocprofvis_dm_topology.cpp Fixed
Comment thread src/model/src/datamodel/rocprofvis_dm_topology.cpp Fixed
Comment thread src/model/src/datamodel/rocprofvis_dm_topology.cpp Fixed
@amokiche-amd amokiche-amd changed the title refactor controller track Refactor controller_track and system_trace Aug 10, 2026
@amokiche-amd
amokiche-amd marked this pull request as ready for review August 10, 2026 15:39
Comment thread src/controller/src/system/rocprofvis_controller_track.cpp Outdated
Comment thread src/controller/src/system/rocprofvis_controller_track.cpp

@drchen-amd drchen-amd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't mind this new approach of using more C++ features and directly calling functions on the objects internally instead of enum. I also do not mind sticking to C style and using enums internally.

I think new approach will lead to some bloating as for each exposed field you end up with an enum version and a direct call version.

If we proceed, this will need more hardening because I only tested for a few minutes and noticed:

  • Queue utilization and counter track stats are broken.
  • Panning and zooming will debug assert: Image

@amokiche-amd
amokiche-amd requested a review from a team as a code owner August 25, 2026 10:56
refactor part of the trace system and track.cpp

get rid of raw pointers in the controller trace

fix destructor

one more fix for ci

Replace unnecasary getters

fix few more geters

one more refactoring

Separate RocPd to the smaller functions

make all getters and setters except objects

remove few heders

Make setter and getter for object private

get back heders

fix linux build

fix cast and get back ""

line endings

line ending
@amokiche-amd
amokiche-amd force-pushed the amokichev/controller-refactoring-one-more-time branch from 6403940 to 0fea075 Compare August 25, 2026 11:00
@amokiche-amd

amokiche-amd commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author
  • Queue utilization and counter track stats are broken.
  • Panning and zooming will debug assert:

Nice catch, thanx queue and counter utilization fixed.
All fixed.

@amokiche-amd

Copy link
Copy Markdown
Contributor Author

I removed the changes with replacement raw pointers to the unique_ptr, because it has unobvious dependency and it work for other separate PR.

Comment thread src/controller/src/system/rocprofvis_controller_topology.cpp
@amokiche-amd
amokiche-amd requested a review from tomk-amd August 27, 2026 18:20
Comment thread src/controller/src/system/rocprofvis_controller_track.cpp Outdated
@amokiche-amd
amokiche-amd merged commit 84d81d3 into main Aug 31, 2026
8 of 9 checks passed
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.

4 participants