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-1507 Improved diffing #1524

Merged
merged 6 commits into from
Oct 23, 2023
Merged

Conversation

maciejburda
Copy link
Member

@maciejburda maciejburda commented Oct 20, 2023

What and why?

It leverages new resourceId field instead of base64 for calculating diff of image wireframes, speeds up the calculation and lowers the CPU pressure.

How?

We added new field to SR schema (that will be later on used for different purposes) and started using this field instead of base64 to calculate object's hash.

Now, on Shopist it goes as low as 12% of CPU for the most complex view on idle. Used to be 60% before the change.

Screenshot 2023-10-23 at 11 40 13

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

Custom CI job configuration (optional)

  • Run unit tests for Core, RUM, Trace, Logs, CR and WVT
  • Run unit tests for Session Replay
  • Run integration tests
  • Run smoke tests
  • Run tests for tools/

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Oct 20, 2023

Datadog Report

Branch report: maciey/RUM-1507-resource-identifier
Commit report: b0fc4b7

dd-sdk-ios: 0 Failed, 0 New Flaky, 241 Passed, 0 Skipped, 4m 48.8s Wall Time

@maciejburda maciejburda marked this pull request as ready for review October 20, 2023 14:48
@maciejburda maciejburda requested a review from a team as a code owner October 20, 2023 14:48
return hash
}

private func computeHash() -> String {
Copy link
Member

Choose a reason for hiding this comment

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

Note

Random question, do we have a benchmark on the improvement this cache is adding vs the time needed to md5 a full image data?

Copy link
Member Author

@maciejburda maciejburda Oct 23, 2023

Choose a reason for hiding this comment

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

Hash is lazily computed once for each resource, so it shouldn't matter that much.

Optimisation of this PR happens in diffing algorithm which takes Hashable protocol into account. In it's implementation we are now ignoring base64 field.

What you're asking here is going to happen in a Resource Endpoint PR where we stop calculating base64 and upload resources directly. In this next PR, I'll measure the difference.

@maciejburda maciejburda merged commit 86cabae into develop Oct 23, 2023
4 checks passed
@maciejburda maciejburda deleted the maciey/RUM-1507-resource-identifier branch October 23, 2023 10:42
@maxep maxep mentioned this pull request Nov 8, 2023
8 tasks
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.

2 participants