Skip to content

Conversation

quic-calvnguy
Copy link
Contributor

@quic-calvnguy quic-calvnguy commented Sep 8, 2025

Description

  • Add optrace profiling level
  • Add profiling to compose graph
  • Add new qnn system profile serializer class
  • Add API versioning safeguards
  • Add backwards compatibility for QNN API < 2.28.1
  • Use QNN System Profile API for QNN API >= 2.28.1
  • Check for log file at end of profiling unit test
  • Ensure system libs are loaded when profiling is enabled

Motivation and Context

Adds optrace level profiling for debugging purposes. Utilizes new QNN System Profile API to generate a binary log file compatible with qnn-profile-viewer executable (or creates a .csv file as before if QNN API version is < 2.28.1).

 - Add optrace profiling level
 - Add profiling to compose graph
 - Add new qnn system profile serializer class
 - Add API versioning safeguards
 - Add backwards compatibility for QNN API < 2.28.1
 - Use QNN System Profile API for QNN API >= 2.28.1
 - Check for log file at end of profiling unit test
 - Ensure system libs are loaded when profiling is enabled
@HectorSVC HectorSVC added the ep:QNN issues related to QNN exeution provider label Sep 8, 2025
@HectorSVC
Copy link
Contributor

/azp run Linux QNN CI Pipeline,Win_TRT_Minimal_CUDA_Test_CI,Windows ARM64 QNN CI Pipeline,Windows GPU Doc Gen CI Pipeline,Windows x64 QNN CI Pipeline

Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@HectorSVC HectorSVC requested a review from Copilot September 8, 2025 22:14
Copy link
Contributor

@Copilot 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 adds optrace profiling level support to the QNN Execution Provider, enabling detailed operation tracing for debugging purposes. It introduces a new QNN System Profile API serializer that generates binary log files compatible with qnn-profile-viewer when using QNN API >= 2.28.1, falling back to CSV format for older versions.

  • Adds optrace profiling level alongside existing basic and detailed levels
  • Implements QNN System Profile API integration with backwards compatibility
  • Adds profiling instrumentation to graph composition, finalization, and execution

Reviewed Changes

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

Show a summary per file
File Description
qnn_basic_test.cc Updates test to use .log extension and adds new optrace test
qnn_execution_provider.cc Adds optrace parsing support to profiling level parser
qnn_utils.h/.cc Adds timestamp utility function for profiling
qnn_profile_serializer.h/.cc New serializer class for QNN System Profile API
qnn_model.cc Adds profiling instrumentation to graph operations
qnn_def.h Defines optrace level and profiling method types
qnn_backend_manager.h/.cc Major refactoring of profiling infrastructure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +4 to +5
#pragma once

Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

This should be an include guard in a .cc file, not a .h file. This line should be removed from the implementation file.

Suggested change
#pragma once

Copilot uses AI. Check for mistakes.

QnnProfile_EventData_t event_data) {
auto system_event = &(event_list.emplace_back());

if (system_event != nullptr) {
Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

This null check is unnecessary as emplace_back() returns a reference to the newly created object, which cannot be null. The check should be removed to improve code clarity.

Copilot uses AI. Check for mistakes.

QnnProfile_ExtendedEventData_t event_data) {
auto system_event = &(event_list.emplace_back());

if (system_event != nullptr) {
Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

This null check is unnecessary as emplace_back() returns a reference to the newly created object, which cannot be null. The check should be removed to improve code clarity.

Copilot uses AI. Check for mistakes.


// This name must be same with the EPContext node name
const auto& graph_name = fused_node.Name();
ORT_RETURN_IF_ERROR(SetGraphInputOutputInfo(graph_viewer, fused_node, logger));
Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

[nitpick] Empty line was removed between logically separate operations. Consider adding the blank line back to improve code readability and maintain separation between setup and construction phases.

Suggested change
ORT_RETURN_IF_ERROR(SetGraphInputOutputInfo(graph_viewer, fused_node, logger));
ORT_RETURN_IF_ERROR(SetGraphInputOutputInfo(graph_viewer, fused_node, logger));

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ep:QNN issues related to QNN exeution provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants