Qualcomm AI Engine Direct - heap profiling at runtime with HTP backend

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic

[jethroqti]

Summary:
Heap profiling at runtime with HTP backend on Android platforms. DSP heap profiling is available for QnnContext_createFromBinary use-cases. It captures total DSP heap usage at two checkpoints:

• Before the first context is created (before_context_created)
• After the last context is freed (after_context_freed)

The difference between the two values represents heap consumed during context execution. The value after freeing is typically equal to or greater than before creation.

Test plan:
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedUtils.test_qnn_backend_runtime_option_heap_profile -b build-android -H ${HOST} -s ${SN} -m ${SOC_MODEL}

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19224

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Run pull request jobs on OSDC runners in shadow mode

⏳ No Failures, 125 Pending

As of commit e304022 with merge base 10c8958 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

This PR is used to measure heap profiling at runtime with HTP backend on Android platforms.
Please take a look. Thanks. @cccclai@haowhsu-quic@winskuo-quic@shewu-quic

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[winskuo-quic]

Hi @abhinaykukkadapu,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[shewu-quic]

Hi @psiddh,
@jethroqti is also one of our contributors. Could you help enabling auto trigger CI for him? Thanks

[psiddh]

@claude review this PR

[claude]

Claude finished @psiddh's task in 4m 13s —— View job

---

PR Review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read changed files
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review test code
• Review documentation
• Post comprehensive review feedback

---

Summary

This PR adds DSP heap profiling support for the HTP backend at runtime, measuring total DSP heap usage at context creation and context free. The implementation involves:

1. Adding a QnnProfile instance to QnnContext (moved from graph-level)
2. Using static mutex/counter to track first-create/last-free across multiple contexts
3. Writing heap usage data to a file via ProfileDataToFile
4. New runtime option QNN_RUNTIME_HEAP_PROFILING_PATH for the output path

Overall the design is sound and follows existing patterns in the codebase. Below are specific findings:

---

Issues

1. Missing --heap_profiling_path flag in executor runner (High)

The test (backends/qualcomm/tests/utils.py:573) passes --heap_profiling_path <filename> as a command-line argument to qnn_executor_runner, but there is no corresponding DEFINE_string(heap_profiling_path, ...) in examples/qualcomm/executor_runner/qnn_executor_runner.cpp. Other runtime options (log_level, profile_level, htp_performance_mode) all have flags defined and wired via backend_options.set_option(...).

Without this, gflags will reject the unknown flag and the runner will abort. The test would fail on device unless a separate unsubmitted runner change handles this.

Fix this →

2. ProfileDataToFile parameter should be const std::string& (Low)

backends/qualcomm/runtime/backends/QnnProfiler.h:30 and .cpp:197:

Qnn_ErrorHandle_t ProfileDataToFile(const std::string profile_filename);

This takes the string by value, causing an unnecessary copy. Should be const std::string&.

3. Race condition risk: WriteHeapProfile reads runtime option without lock (Medium)

QnnContextCommon.cpp:20-38 — WriteHeapProfile() calls get_runtime_option(QNN_RUNTIME_HEAP_PROFILING_PATH, ...) which reads from QnnExecuTorchBackend's member qnn_runtime_heap_profiling_path_. The set_option method uses runtime_option_mutex_ for writes, but get_option (called here indirectly) does not hold the lock for reads. If set_option is called concurrently from another thread while a context is being destroyed, there could be a data race. While unlikely in practice (profiling path is typically set once before init), worth noting.

4. count_num_events vs num_events check doesn't use error correctly (Low)

QnnProfiler.cpp:252:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If all events are iterated without matching the heap identifiers, error holds the result of the last qnn_profile_get_event_data call — which was successful (otherwise we'd have returned earlier). So this is fine functionally, but returning QNN_SUCCESS explicitly would be clearer than relying on error holding a stale success value.

5. File truncation ordering assumption (Medium)

QnnProfiler.cpp:234:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
}

This assumes the "before_context_created" event always appears before the "after_context_freed" event in the profile events array. If QNN ever returns them in a different order, the file would be truncated after writing the first entry. Consider writing both values to a buffer and writing the file once at the end, or always truncating at the start of ProfileDataToFile before iterating.

6. Destructor ordering concern (Low)

In QnnContext::~QnnContext(), after the lock guard scope ends at line 54 but before qnn_context_free is called, another thread could decrement the counter in its own destructor. This seems safe due to the do_heap_profile local variable capturing the decision, but it's worth a comment explaining the lock is only held briefly to read/update the counter.

---

Suggestions (Non-blocking)

1. Documentation references self.chipset_table — The README code snippet (backends/qualcomm/debugger/README.md:262) uses self.chipset_table[TestQNN.soc_model] which is test-internal. For external users, showing a QcomChipset.SM8850 example would be more helpful.

2. Consider asserting difference >= 0 in the test validation (backends/qualcomm/tests/utils.py:402). The PR description says "after freeing is typically equal to or greater than before creation" — if the reverse happens, it might indicate a measurement error worth flagging.

3. Unused variable — backends/qualcomm/runtime/backends/QnnContextCommon.h:98: profile_level_ is stored but only used to construct qnn_profiler_ and check need_to_profile_. You could remove it and pass directly.

4. GpuContext and LpaiContext always pass QnnExecuTorchProfileLevel::kProfileOff to QnnContext, so is_htp_backend_ will always be false for them and need_to_profile_ will be false. The added profiler allocation is therefore wasted for GPU/LPAI. Consider making the profiler creation conditional.

---

Positive Notes

• The mutex + counter pattern for tracking first-create/last-free across multiple contexts is correctly implemented with proper rollback on error.
• The separation of concerns (profile handle management in QnnContext, file writing in QnnProfiler) is clean.
• The FetchEvents refactor reducing duplication in QnnProfiler.cpp is a welcome improvement.
• README documentation is thorough with example output.

---

[jethroqti]

@psiddh Thanks for this review. I will handle ASAP.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[psiddh]

@claude re-review the PR again thoroughly

[psiddh]

@jethroqti Looks like this branch has few merge conflicts

[claude]

Claude finished @psiddh's task in 2m 26s —— View job

---

Re-review: Qualcomm AI Engine Direct - Heap Profiling at Runtime

• Gather context and read all changed files
• Check if previous review issues were addressed
• Review runtime implementation (QnnBackendOptions, QnnContextCommon, QnnProfiler)
• Review backend-specific changes (HTP, GPU, LPAI)
• Review executor runner changes
• Review test code and documentation
• Post comprehensive review feedback

---

Previous Review Issues — Status

#	Issue	Status
1	Missing --heap_profiling_path flag in executor runner	Fixed ✓
2	ProfileDataToFile parameter should be const std::string&	Fixed ✓
3	Race condition: get_option without lock	Fixed ✓ (lock added)
4	Return value clarity in ProfileDataToFile	Still present (see below)
5	File truncation ordering assumption	Still present (see below)
6	Destructor ordering concern	Still present (acceptable)

---

Remaining Issues

1. File truncation depends on event ordering assumption (Medium)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:233-241:

if (strcmp(event_data.identifier, DEFINE_HEAP_BEFORE_CREATION) == 0) {
open_mode = std::ios::trunc;
} elseif (strcmp(event_data.identifier, DEFINE_HEAP_AFTER_FREED) == 0) {
open_mode = std::ios::app;
} else {
count_num_events++;
continue;
}

This assumes QNN always returns the "before_context_created" event before the "after_context_freed" event. If the order is ever reversed, the "before" value written first would be lost when "after" truncates the file.

Recommendation: Always truncate at the beginning of ProfileDataToFile (before the loop), and always append within the loop. This makes the output correct regardless of event ordering.

Fix this →

2. error return value at end of ProfileDataToFile is ambiguous (Low)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:252-257:

if (count_num_events == num_events) {
QNN_EXECUTORCH_LOG_WARN(
"Not HTP backend but enable htp profiling. Please check setting.");
returnQNN_SUCCESS;
}
return error;

If num_events > 0 and heap events are found, error holds the result of the last qnn_profile_get_event_data call — which is QNN_SUCCESS (or we'd have returned earlier). This is technically correct but unclear to readers. Consider explicitly returning QNN_SUCCESS instead of error for clarity.

3. [[maybe_unused]] on profile_level_ and context_ in HtpContextCustomConfig.h (Low)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:38,46:

[[maybe_unused]] QnnExecuTorchProfileLevel profile_level_;
...
[[maybe_unused]]const QnnContext* context_;

profile_level_ is now used in the target build (target/HtpContextCustomConfig.cpp:23) but not in the host build (host/HtpContextCustomConfig.cpp). Using [[maybe_unused]] on a member that IS used in one build configuration is correct but unusual. Consider adding a brief comment to explain the host/target split, or use #ifdef for documentation purposes.

Similarly, context_ is used in the target build (cast to HtpContext*) but not host. The [[maybe_unused]] is correct.

4. Test validation doesn't assert difference >= 0 (Low)

backends/qualcomm/tests/utils.py:400-406:

before, after=valuesdifference=after-beforeprint(f"before_context_created: {before} bytes")
print(f"after_context_freed: {after} bytes")
print(f"difference: {difference:.2f} bytes")

The test prints the difference but never asserts that after >= before (or difference >= 0). Per the PR description, "the value after freeing is typically equal to or greater than before creation." If it's ever negative, that likely indicates a measurement error worth failing on.

Recommendation: Add self.assertGreaterEqual(after, before, "after_context_freed should be >= before_context_created").

Fix this →

5. WriteHeapProfile called twice — at creation AND destruction (Design Question)

The PR description says heap profiling captures values at two checkpoints, but looking at the implementation, WriteHeapProfile() is called:

• After the first context is created (QnnContextCommon.cpp:128-129)
• After the last context is freed (QnnContextCommon.cpp:64-65)

Each call writes to the same file. The "before_context_created" event is only present in the profile collected at creation time, and "after_context_freed" only at destruction time. This means:

• At creation, the file is written with only "before_context_created" (truncate mode)
• At destruction, "after_context_freed" is appended

This is coherent but relies on:

1. QNN returning only the relevant event at each checkpoint
2. The file persisting between the two writes

If QNN returns both events at each checkpoint, the file could contain duplicates. The code's filtering logic handles this correctly since it only writes events matching the two known identifiers, but worth documenting this assumption.

---

New Observations

6. File opened/closed per event line (Low, Performance)

backends/qualcomm/runtime/backends/QnnProfiler.cpp:244:

std::ofstream ofs(profile_filename, open_mode);

Inside the loop, a new ofstream is opened for each matching event. Since there are at most 2 matching events per call, this isn't a performance concern, but it's unusual. Opening the file once before the loop and writing all matching events would be slightly cleaner.

7. use_multi_contexts=True required but not validated at runtime (Low)

The test sets use_multi_contexts=True which is necessary for the multi-context flow that splits the model and creates multiple QNN contexts. The heap profiling logic (first-create/last-free counting) is specifically designed for this scenario. However, if a user enables heap profiling without use_multi_contexts, the counter will still increment/decrement with a single context — it'll work correctly but the "first/last" terminology is misleading for single-context usage.

8. profile_level_ initialization order in HtpContextCustomConfig (Nit)

backends/qualcomm/runtime/backends/htp/HtpContextCustomConfig.h:31-33:

: profile_level_(profile_level),
context_(context),
htp_options_(htp_options) {}

Member initializer order matches declaration order (profile_level_ declared at line 38, then context_ at line 46, then htp_options_ at line 49). This is correct — good practice.

---

Positive Notes

• The fix commits properly addressed the critical issues from the first review (executor runner flag, const ref, get_option locking, unused member removal).
• The FetchEvents refactor cleanly deduplicates event fetching logic between ProfileData and ProfileDataToFile.
• The static mutex + counter pattern correctly handles the multi-context first-create/last-free lifecycle.
• Rollback logic on context creation failure (QnnContextCommon.cpp:120-126) is correct.
• The QNN_HTP_CONTEXT_CONFIG_OPTION_DSP_MEMORY_PROFILING_ENABLED context custom config is correctly placed in the target-only build file.
• README documentation is clear with example output.

---

Summary

The PR is in good shape after the review fixes. The remaining issues are mostly low severity (file ordering assumption, missing test assertion, style nits). Issue #1 (file truncation ordering) and Issue #4 (missing test assertion) are the most impactful to address before merge. The rest can be taken as suggestions.

---

[jethroqti]

@jethroqti Looks like this branch has few merge conflicts

It seems one README file. I will check.

[jethroqti]

@pytorchbot label "release notes: qualcomm"

[jethroqti]

@psiddh
Thanks for this review.
About the issue 1. File truncation depends on event ordering assumption (Medium):
I need to follow the order to get profiling information. If I follow the recommendation, I cannot get the correct information.
About issues 2-4 are resolved.

@shewu-quic