Skip to content

[major] Refactor performance into core service#1628

Open
cfernald wants to merge 21 commits into
OpenDevicePartnership:majorfrom
cfernald:performance_refactor
Open

[major] Refactor performance into core service#1628
cfernald wants to merge 21 commits into
OpenDevicePartnership:majorfrom
cfernald:performance_refactor

Conversation

@cfernald

@cfernald cfernald commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

This PR addresses multiple issues with the current performance structure

  1. Performance data is not available until the component is dispatched. This can cause significant misrepresentation of the DXE execution time and cause events from images to be lost if not dispatched in the first loop.
  2. Performance implementations in the SDK require SDK statics, which is generally an SDK antipattern.

This PR does the following

  1. Moves the core performance implementation into the core, into a new struct names CorePerformance, following the pattern established by the timer and memory management.
  2. Creates a new service trait PerfortmanceMeasurement that exposes this core support.
  3. Updates the component to now use this core service to produce the protocol & publish tables.
  4. Change initialization and configuration of performance measurements to be done by the core.
  5. Updates all documentation according to these changes
  6. Some smaller rework and restructuring

Resolves: #1483

Reviewing this PR

This PR is intended to be merged as a single commit. The individual commits do attempt to follow a basic progression of the code rework but may be incomplete at the partial steps. Recommended review flow

  1. Start with the new theory of operation md (performance_measurement.md) to understand the intended structure.
  2. Review the new service interface (PerformanceMeasurement)
  3. Review the new core implementation
  4. Review the changes to the component to consume the new service
  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

Tested for consistent performance data using dp before and after refactor.

Integration Instructions

Platforms must change use of with_measurements from the patina_performance to instead override PlatformInfo::default_performance_config()

@patina-automation

patina-automation Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

⌛ QEMU Validation Pending

QEMU validation is pending on successful CI completion.

Note: Any previous results are available in this comment's edit history.

This comment was automatically generated by the Patina QEMU PR Validation workflow.

@github-actions github-actions Bot added impact:breaking-change Requires integration attention impact:testing Affects testing type:documentation Improvements or additions to documentation labels Jul 8, 2026
Comment thread patina_dxe_core/src/lib.rs Outdated
@cfernald cfernald marked this pull request as ready for review July 8, 2026 22:18
@cfernald cfernald linked an issue Jul 8, 2026 that may be closed by this pull request
1 task
Comment thread components/patina_performance/README.md Outdated
@makubacki

Copy link
Copy Markdown
Collaborator

I haven't taken a look through this one yet, but similar to #1630, please try to keep each file about 80% (ideally 85%) coverage.

@cfernald

Copy link
Copy Markdown
Contributor Author

@makubacki @Javagedes , i will add tests, but please review the changes for the new structure. The PR comment has a review guide since the commits are a bit of a mess due to the complexity of the refactor.

Comment thread patina_dxe_core/src/performance.rs Outdated
makubacki
makubacki previously approved these changes Jul 13, 2026
Comment thread patina_dxe_core/src/performance.rs Outdated
Comment thread patina_dxe_core/src/lib.rs Outdated
Comment thread patina_dxe_core/src/lib.rs Outdated
@Javagedes

Copy link
Copy Markdown
Contributor

I will be reviewing this today, please do not merge :)

Comment thread docs/src/dxe_core/performance_measurement.md
Comment thread docs/src/integrate/dxe_core.md Outdated
Comment thread sdk/patina/src/component/service/performance.rs Outdated
Comment thread sdk/patina/src/component/service/performance.rs Outdated
Comment thread sdk/patina/src/performance/config.rs
Comment thread sdk/patina/src/performance.rs
Comment thread patina_dxe_core/src/performance.rs Outdated
@cfernald cfernald force-pushed the performance_refactor branch from c02bac4 to 3c2e237 Compare July 13, 2026 22:22
@makubacki makubacki changed the title Refactor performance into core service [major] Refactor performance into core service Jul 14, 2026
@makubacki

Copy link
Copy Markdown
Collaborator

Note: Added [major] prefix to PR title per RFC: Patina Release Process.

@makubacki makubacki dismissed their stale review July 14, 2026 22:59

Going to review again after refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:breaking-change Requires integration attention impact:testing Affects testing type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Performance tracking has a blackout as DXE entry

5 participants