Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-7180 SwiftUI Benchmark Scenario #2146

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

maxep
Copy link
Member

@maxep maxep commented Dec 13, 2024

What and why?

Add a Session Replay SwiftUI scenario for continuous benchmarking.

Note: Everything under BenchmarkTests/SwiftUICatalog is 3rd party.

How?

Integrate with the third-party SwiftUICatalog in a similar fashion than UIKitCatalog.

The scenario will go through multiple SwiftUI components to measure pre-defined metrics.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs (see our guidelines [internal]) and run make api-surface)

@maxep maxep requested review from a team as code owners December 13, 2024 16:14
@maxep maxep force-pushed the maxep/RUM-7180/swiftui-benchmark-scenario branch from 55afe0e to 8d94343 Compare December 13, 2024 16:20
@maxep maxep marked this pull request as draft December 13, 2024 16:24
@ncreated
Copy link
Member

@maxep Would it be possible to consider bundling the images differently, skipping some, or resizing them to help reduce the impact on the repository size? The total size of the images in this PR is 1.5MB, which adds over 10% to the current size of the repository (<15MB).

@maxep
Copy link
Member Author

maxep commented Dec 16, 2024

@ncreated Yes, totally 👍

simaoseica-dd
simaoseica-dd previously approved these changes Dec 16, 2024
BenchmarkTests/BenchmarkTests.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
@simaoseica-dd simaoseica-dd self-requested a review December 16, 2024 12:09
@maxep maxep force-pushed the maxep/RUM-7180/swiftui-benchmark-scenario branch 6 times, most recently from d6ad6aa to 6698ac0 Compare December 16, 2024 15:48
@maxep maxep marked this pull request as ready for review December 16, 2024 15:59
@maxep
Copy link
Member Author

maxep commented Dec 16, 2024

Thank you for the initial review! I apology for such a large PR, most of it is from SwiftUICatalog which was convenient to build an app that covers a large set of SwiftUI component. I'm not expecting a review on this framework implementation, The only custom part is BenchmarkTests/SwiftUICatalog/DatadogMonitor.swift which is used to instrument RUM views.

@maxep maxep force-pushed the maxep/RUM-7180/swiftui-benchmark-scenario branch from 6698ac0 to f2d89b9 Compare December 17, 2024 13:06
ncreated
ncreated previously approved these changes Dec 18, 2024
Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Looks good 👌. My only ask is to update the LICENSE-3rdparty.csv. I think it should also mention the UIKitCatalog.

BenchmarkTests/SwiftUICatalog/LICENSE Outdated Show resolved Hide resolved
Comment on lines 1200 to 1972
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
IPHONEOS_DEPLOYMENT_TARGET = 17.0;
Copy link
Member

Choose a reason for hiding this comment

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

suggestion/ We have two xcconfig files set up for Benchmarks project, this setting could easily go there to simplify configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

The xcconfig is currently applied to the Runner target only.
What I did instead is set the IPHONEOS_DEPLOYMENT_TARGET at the project level and remove overrides in the targets.

@maxep maxep force-pushed the maxep/RUM-7180/swiftui-benchmark-scenario branch from 7d6803b to 5a193aa Compare December 18, 2024 11:33
@maxep maxep requested review from ncreated and mariedm December 18, 2024 11:39
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Dec 18, 2024

Datadog Report

Branch report: maxep/RUM-7180/swiftui-benchmark-scenario
Commit report: d868115
Test service: dd-sdk-ios

✅ 0 Failed, 553 Passed, 3057 Skipped, 19.3s Total duration (2m 11.97s time saved)

@maxep maxep force-pushed the maxep/RUM-7180/swiftui-benchmark-scenario branch from 1ab0d94 to d868115 Compare December 18, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants