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

[FIX] - replace lodash merge with recursive assign 2 #48

Merged
merged 7 commits into from
Apr 5, 2020

Conversation

alonfixler
Copy link
Contributor

@alonfixler alonfixler commented Apr 2, 2020

This reverts 39f208a and replaces usage of lodash merge with version 2 of recursive assign.

lodash has performance issues:
lodash/lodash#1865
reduxjs/redux#1262 (comment)

Since this change deploys to gig page perseus raise the cpu dramatically which result in massive amount of timeouts. (In the image below you can see the results of 2 faulty deploys):
Screen Shot 2020-04-02 at 15 28 41

I isolated all the dependencies that were added in recent days in gig page and tested on dev the following PR which explicitly add version 1.8.2 of i18n to gig page. When deploying in dev I've noticed an increment in cpu (the orange line) which went down only after deploying a different tag (the yellow tag):
Screen Shot 2020-04-02 at 13 24 04

@michael5r
Copy link

That's some nice detective work, Alon.

@alonfixler alonfixler changed the title [REVERT] - usage of lodash merge [FIX] - replace lodash merge with recursive assign Apr 2, 2020
@alonfixler alonfixler changed the title [FIX] - replace lodash merge with recursive assign [FIX] - replace lodash merge with recursive assign 2 Apr 2, 2020
@ghost
Copy link

ghost commented Apr 3, 2020

Congratulations 🎉. DeepCode analyzed your code in 1.105 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard

Copy link
Contributor

@raphaelboukara raphaelboukara left a comment

Choose a reason for hiding this comment

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

❤️

@alonfixler alonfixler merged commit e553fb7 into master Apr 5, 2020
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