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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: use native Object.assign (refs #6407) #6832

Merged
merged 1 commit into from Aug 3, 2016
Merged

Chore: use native Object.assign (refs #6407) #6832

merged 1 commit into from Aug 3, 2016

Conversation

gyandeeps
Copy link
Member

@gyandeeps gyandeeps commented Aug 2, 2016

What issue does this pull request address?
Use native function rather than a polyfill from lodash. This way we are more native bound than library bound 馃槃

What changes did you make? (Give an overview)
Replaced lodash.assign with native Object.assign. As a result of this i was able to get rid of lodash in many modules.

Is there anything you'd like reviewers to focus on?

nothing specific

@eslintbot
Copy link

LGTM

@ilyavolodin
Copy link
Member

@gyandeeps Can you run perf test before and after for this change please?

@gyandeeps
Copy link
Member Author

gyandeeps commented Aug 2, 2016

@ilyavolodin : when i made this change performance was not the motivation 馃槃 . but i guess there is no difference based on the numbers.

Before

Loading:
  Load performance Run #1:  285.152035ms
  Load performance Run #2:  293.246393ms
  Load performance Run #3:  286.062485ms
  Load performance Run #4:  288.535803ms
  Load performance Run #5:  277.653648ms

  Load Performance median:  286.062485ms


Single File:
  CPU Speed is 2793 with multiplier 13000000
  Performance Run #1:  3404.311078ms
  Performance Run #2:  3374.370675ms
  Performance Run #3:  3362.172693ms
  Performance Run #4:  3387.370983ms
  Performance Run #5:  3407.566561ms

  Performance budget ok:  3387.370983ms (limit: 4654.493376297887ms)


Multi Files (0 files):
  CPU Speed is 2793 with multiplier 39000000
  Performance Run #1:  944.787268ms
  Performance Run #2:  1000.370191ms
  Performance Run #3:  1036.133591ms
  Performance Run #4:  1156.578407ms
  Performance Run #5:  1120.515556ms

  Performance budget ok:  1036.133591ms (limit: 13963.480128893663ms)

After

Loading:
  Load performance Run #1:  283.44549ms
  Load performance Run #2:  281.234239ms
  Load performance Run #3:  285.52076ms
  Load performance Run #4:  290.018767ms
  Load performance Run #5:  279.19379ms

  Load Performance median:  283.44549ms


Single File:
  CPU Speed is 2793 with multiplier 13000000
  Performance Run #1:  3345.934495ms
  Performance Run #2:  3364.047106ms
  Performance Run #3:  3374.118139ms
  Performance Run #4:  3369.067411ms
  Performance Run #5:  3377.237274ms

  Performance budget ok:  3369.067411ms (limit: 4654.493376297887ms)


Multi Files (0 files):
  CPU Speed is 2793 with multiplier 39000000
  Performance Run #1:  901.099578ms
  Performance Run #2:  900.602568ms
  Performance Run #3:  901.339652ms
  Performance Run #4:  894.463259ms
  Performance Run #5:  893.263986ms

  Performance budget ok:  900.602568ms (limit: 13963.480128893663ms)

@ilyavolodin
Copy link
Member

LGTM, but waiting another day for others to look

@alberto
Copy link
Member

alberto commented Aug 3, 2016

LGTM

@alberto alberto merged commit cdded07 into master Aug 3, 2016
@gyandeeps gyandeeps deleted the obj-assign branch August 3, 2016 15:28
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants