Side navigation
#9994 closed enhancement (wontfix)
Opened August 07, 2011 10:53AM UTC
Closed August 07, 2011 02:56PM UTC
Last modified November 23, 2011 05:05PM UTC
Optimization improvement in jQuery.extend
| Reported by: | anonymous | Owned by: | rwaldron |
|---|---|---|---|
| Priority: | low | Milestone: | |
| Component: | core | Version: | 1.6.2 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
On line 313 of core.js:
https://github.com/jquery/jquery/blob/master/src/core.js#L313
there is a check for:
typeof target === "boolean"
However, target has been set by:
target = arguments[0] || {},
so in case it's a boolean, it's always true (if arguments[0] was false, target is {}). Checking for true is much faster than a typeof check and then comparing strings, especially on IE:
http://jsperf.com/true-vs-typeof
Thanks for considering this optimization.
Attachments (0)
Change History (9)
Changed August 07, 2011 12:11PM UTC by comment:1
| _comment0: | The point → 1312719095713324 |
|---|---|
| component: | unfiled → core |
| milestone: | None → 1.6.3 |
| owner: | → rwaldron |
| priority: | undecided → low |
| status: | new → assigned |
Changed August 07, 2011 01:21PM UTC by comment:2
Seems to me that this will not set the target correctly if both false and a target is passed to extend. That may not happen very often, but would be a regression.
$.extend( false, { some: "target" }, { more: "stuff" } );
Changed August 07, 2011 01:44PM UTC by comment:3
More often than I would have expected, that's fer sure.
http://google.com/codesearch#search/&q=\\$\\.extend\\(\\s*false
There are probably more of the form $.extend(iCanHazDeep, defaults, options) but they're too hard to grep.
Changed August 07, 2011 01:51PM UTC by comment:4
If arguments[0] is any kind of falsy, it's not boolean by the time it reaches that condition.
Changed August 07, 2011 01:53PM UTC by comment:5
Sorry, by "it" I meant the target var
Changed August 07, 2011 02:56PM UTC by comment:6
| resolution: | → wontfix |
|---|---|
| status: | assigned → closed |
Turns out this is a performance hit, so no thanks.
Changed August 07, 2011 05:55PM UTC by comment:7
This is now neither here nor there, but let me explain further. I don't think this would work anyway because target needs to get assigned to arguments[1] even if arguments[0] is false. It can't just be an empty plain object. The only place that happens is from within the typeof conditional, so it can't change unless that assignment is added somewhere else and at that point we're talking about changing more than just the conditional.
Changed August 26, 2011 01:34AM UTC by comment:8
| milestone: | 1.6.3 |
|---|