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

Core: Tiny efficiency fix to jQuery.extend / jQuery.fn.extend #4246

Merged
merged 1 commit into from Dec 12, 2018

Conversation

Projects
None yet
4 participants
@marjakh
Contributor

marjakh commented Nov 29, 2018

Summary

Read target[name] only when it's needed.

In addition to doing the property read only when needed, this avoids a slow path
in V8 (see issue for more details).

Fixes #4245

Checklist

@mgol

mgol approved these changes Nov 29, 2018

Thanks for a detailed bug report and a PR! This looks good to me both code-wise and taking into account the description of V8 internals. It also doesn't look like it could cause regressions in another browser.

It also doesn't change the gzipped minified size.

@mgol mgol added the Needs review label Nov 29, 2018

@gibson042

Thanks for the contribution! The change looks good to me.

Show resolved Hide resolved src/core.js Outdated
Core: Tiny efficiency fix to jQuery.extend / jQuery.fn.extend
Read target[name] only when it's needed.

In addition to doing the property read only when needed, this avoids a slow path
in V8 (see issue for more details).

Fixes #4245

@marjakh marjakh force-pushed the marjakh:tiny-fix-extend branch from ab2e7de to ac622f1 Nov 30, 2018

@marjakh

This comment has been minimized.

Contributor

marjakh commented Nov 30, 2018

Added the suggested changes; will the Travis CI do a new run automatically or what needs to be done to kick it off?

@gibson042

This comment has been minimized.

Member

gibson042 commented Nov 30, 2018

It's all set! We'll probably land this within a week.

@timmywil

LGTM

@timmywil timmywil removed the Needs review label Dec 10, 2018

@mgol mgol merged commit 4ffb1df into jquery:master Dec 12, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@mgol

This comment has been minimized.

Member

mgol commented Dec 12, 2018

Landed, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment