Skip to content

Fix: key swimlane dicts by (task_id, core_id) for mixed_task support#698

Merged
ChaoZheng109 merged 1 commit intohw-native-sys:mainfrom
indigo1973:prof_0428_v2
Apr 28, 2026
Merged

Fix: key swimlane dicts by (task_id, core_id) for mixed_task support#698
ChaoZheng109 merged 1 commit intohw-native-sys:mainfrom
indigo1973:prof_0428_v2

Conversation

@indigo1973
Copy link
Copy Markdown
Contributor

@indigo1973 indigo1973 commented Apr 28, 2026

mixed_task dispatches a single task_id to multiple cores, producing
multiple perf records that share the same task_id. The previous code
used task_id alone as dict key for task_to_event_id, task_to_aicpu_tid,
and task_to_aicpu_event_id, so later per-core records silently overwrote
earlier ones — wrong AICPU View lane assignments and mislinked flow
arrows.

Additionally, task_map was a single-value dict so flow arrows between
two mixed_tasks all converged on one arbitrary destination core instead
of fanning out to every core of the successor task.

Changes:

  • Switch all three event/tid dicts to (task_id, core_id) tuple keys
  • Convert task_map to defaultdict(list) for one-to-many lookup
  • Expand both flow event loops (AICore View + AICPU View) to iterate
    all destination core records per successor task_id

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the swimlane_converter.py tool to use a composite key of (task_id, core_id) instead of just task_id for several internal mapping dictionaries. This change aims to better handle multi-core tasks where task IDs might not be unique. However, the review identifies a significant remaining issue: the task_map used to resolve task dependencies still uses task_id as its primary key, which could lead to incorrect core identification and broken flow arrows for multi-core kernels.

Comment thread simpler_setup/tools/swimlane_converter.py Outdated
@indigo1973 indigo1973 changed the title Fix: use (task_id, core_id) as dict key in swimlane trace generation Fix: key swimlane dicts by (task_id, core_id) for mixed_task support Apr 28, 2026
@indigo1973
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the swimlane_converter.py script to handle tasks with duplicate IDs by using a tuple of (task_id, core_id) as a unique identifier in mapping dictionaries. It also modifies task_map to store lists of tasks per ID to accommodate these duplicates. The review feedback suggests refining the type hinting for the new task_map structure to improve static analysis and optimizing performance by moving loop-invariant calculations outside of nested loops.

Comment thread simpler_setup/tools/swimlane_converter.py
Comment thread simpler_setup/tools/swimlane_converter.py Outdated
@indigo1973
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the swimlane_converter.py tool to support scenarios where task IDs are not unique across cores by utilizing a tuple of (task_id, core_id) for dictionary lookups. The task_map structure is refactored to store lists of tasks, and the flow event generation logic is updated to iterate through all instances of a successor task ID. A review comment suggests using the task object's ID field for consistency when generating AICPU view flow events.

Comment thread simpler_setup/tools/swimlane_converter.py Outdated
mixed_task dispatches a single task_id to multiple cores, producing
multiple perf records that share the same task_id. The previous code
used task_id alone as dict key for task_to_event_id, task_to_aicpu_tid,
and task_to_aicpu_event_id, so later per-core records silently overwrote
earlier ones — wrong AICPU View lane assignments and mislinked flow
arrows.
Additionally, task_map was a single-value dict so flow arrows between
two mixed_tasks all converged on one arbitrary destination core instead
of fanning out to every core of the successor task.
Changes:
Switch all three event/tid dicts to (task_id, core_id) tuple keys
Convert task_map to defaultdict(list) for one-to-many lookup
Expand both flow event loops (AICore View + AICPU View) to iterate
all destination core records per successor task_id
@indigo1973
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the swimlane_converter.py tool to handle non-unique task IDs by using a composite key of (task_id, core_id) for event and thread mappings. Additionally, task_map has been refactored to store lists of tasks per ID, and the flow event generation logic now iterates through all tasks associated with successor IDs to ensure correct dependency visualization. I have no feedback to provide as there are no review comments.

@ChaoZheng109 ChaoZheng109 merged commit e7eb6b2 into hw-native-sys:main Apr 28, 2026
14 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.

2 participants