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

Switching "deep-equal" package for "fast-deep-equal". #1665

Closed

Conversation

Davy803
Copy link
Contributor

@Davy803 Davy803 commented Nov 16, 2023

Description / Motivation

We were noticing that deepEqual in componentDidUpdate of SitecoreContext was taking up a significant chunk of time for large Layout Service payloads.

The package "fast-deep-equal" shows 100x faster performance in benchmarks compared to "deep-equal".

This was the only usage of the "deep-equal" package, so we were able to replace the dependency entirely.

Testing Details

  • Unit Test Added
  • Manual Test/Other (Please elaborate)

There is no functional change, only performance change, so no new unit tests. We have implemented this change on 2 different projects and have not seen any issues, just faster performance.

In our Lighthouse tests for one of our projects, we saw the Total Blocking Time reduced from 1090ms to 20ms in Desktop, and from 5170ms to 330ms in mobile. The results were reproducible, and we saw significant gains in a different project.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Not technically a bug, but it's not new functionality either.

…nificant performance boost for Total Blocking Time, especially for large layout service payloads.
@illiakovalenko
Copy link
Contributor

@Davy803 Thank you for submitting your contribution! I've added a ticket containing all the details, which has been added to our internal backlog for thorough review

@illiakovalenko illiakovalenko added the backlog Issue/PR/discussion is reviewed and added to backlog for the further work label Nov 20, 2023
@yavorsk yavorsk self-assigned this Jan 19, 2024
illiakovalenko added a commit that referenced this pull request Jan 22, 2024
…l" - based on #1665 (#1719)

* Switching "deep-equal" package for "fast-deep-equal".  This gave a significant performance boost for Total Blocking Time, especially for large layout service payloads.

* update yarn.lock

* update yarn.lock

* update changelog

* use latest version of fast-deep-equal; @remove types/deep-equal

* fix changelog entry

* Minor update to CHANGELOG.md

* Update CHANGELOG.md

---------

Co-authored-by: David <[email protected]>
Co-authored-by: Illia Kovalenko <[email protected]>
illiakovalenko added a commit that referenced this pull request Jan 22, 2024
…ual" - based on #1665 (#1719)

* Switching "deep-equal" package for "fast-deep-equal".  This gave a significant performance boost for Total Blocking Time, especially for large layout service payloads.

* update yarn.lock

* update yarn.lock

* update changelog

* use latest version of fast-deep-equal; @remove types/deep-equal

* fix changelog entry

* Minor update to CHANGELOG.md

* Update CHANGELOG.md

---------

Co-authored-by: David <[email protected]>
Co-authored-by: Illia Kovalenko <[email protected]>
@illiakovalenko
Copy link
Contributor

Hey @Davy803
We pulled your changes in #1719 and added some minor things, it's merged to canary.
We greatly appreciate your contribution! Your efforts help improve our project. Thank you!
I close this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issue/PR/discussion is reviewed and added to backlog for the further work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants