Skip to content

CHANGE: Use ProfilerMarker in place of Profiler based on Unity version and presence of com.unity.profiling.core package#1970

Merged
stefanunity merged 12 commits intodevelopfrom
profiler/sampler-to-marker
Jul 30, 2024
Merged

CHANGE: Use ProfilerMarker in place of Profiler based on Unity version and presence of com.unity.profiling.core package#1970
stefanunity merged 12 commits intodevelopfrom
profiler/sampler-to-marker

Conversation

@surfnerd
Copy link
Copy Markdown
Collaborator

@surfnerd surfnerd commented Jul 18, 2024

Description

Use ProfilerMarker instead of Profiler.[Begin|End]Sample when the com.unity.profiling.core package is present at version 1.0.2 and the Unity version is 2020.3 or greater.

Changes made

A wrapper class, InputProfilerMarker, was added to encapsulate the #ifdef logic for using ProfilerMaker vs. Profiler. Most places that used to call Profiler now call InputProfilerMarker.

Testing

I ran a handful of tests on Yamato. The code analyzer test made me realize that I need to conditionally compile my changes based on the Unity Version and the presence of the com.unity.profiling.core package.

I also tested to ensure that the markers show up in the Profiler Window Timeline for both cases on a test project.

Risk

I believe the risk of this change is low as there are no feature changes or logic changes; it just replaces one profiler call with another based on the Unity version and package presence.

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
    • Jira port for the next release set as "Resolved".
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

@unity-cla-assistant
Copy link
Copy Markdown

unity-cla-assistant commented Jul 18, 2024

CLA assistant check
All committers have signed the CLA.

@surfnerd surfnerd changed the title Use ProfilerMarker in place of Profiler based on Unity version and presence of com.unity.profiling.core package CHANGE: Use ProfilerMarker in place of Profiler based on Unity version and presence of com.unity.profiling.core package Jul 18, 2024
@surfnerd surfnerd force-pushed the profiler/sampler-to-marker branch from 0ee8084 to 0b8ff75 Compare July 18, 2024 21:56
@surfnerd surfnerd force-pushed the profiler/sampler-to-marker branch from 0b8ff75 to 75e445c Compare July 18, 2024 21:57
@surfnerd surfnerd marked this pull request as ready for review July 29, 2024 21:40
@surfnerd surfnerd requested a review from stefanunity July 29, 2024 21:42
@surfnerd surfnerd added the waiting-for-review The issue is scheduled to be reviewed by the Unity maintainers label Jul 29, 2024
Copy link
Copy Markdown
Collaborator

@stefanunity stefanunity left a comment

Choose a reason for hiding this comment

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

LGTM

@stefanunity stefanunity merged commit a4285dc into develop Jul 30, 2024
@stefanunity stefanunity deleted the profiler/sampler-to-marker branch July 30, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-review The issue is scheduled to be reviewed by the Unity maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants