Skip to main content

Bug Tracker

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 rwaldron comment:1

_comment0: The point 1312719095713324
component: unfiledcore
milestone: None1.6.3
owner: → rwaldron
priority: undecidedlow
status: newassigned

Changed August 07, 2011 01:21PM UTC by timmywil 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 dmethvin 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 rwaldron 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 rwaldron comment:5

Sorry, by "it" I meant the target var

Changed August 07, 2011 02:56PM UTC by rwaldron comment:6

resolution: → wontfix
status: assignedclosed

Turns out this is a performance hit, so no thanks.

Changed August 07, 2011 05:55PM UTC by timmywil 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 dmethvin comment:8

milestone: 1.6.3

Changed November 23, 2011 05:05PM UTC by rwaldron comment:9

#10867 is a duplicate of this ticket.