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

jQuery.extend / jQuery.fn.extend triggers slow path in V8 (and it looks like easy to fix) #4245

Closed
marjakh opened this Issue Nov 29, 2018 · 0 comments

Comments

Projects
None yet
2 participants
@marjakh
Copy link
Contributor

commented Nov 29, 2018

Description

In V8, adding properties into the prototype chain, interleaved with property access, results in copying fast mode maps repeatedly. (A "map" describes the internal layout of an object.)

Example:

function Foo() { ... }

// Add properties (prototype in setup stage)
Foo.prototype.a = function() { ... }
Foo.prototype.b = function() { ... }

let f = new Foo();
for (let i = 0; i < 100; ++i) {
f.a; // Second access from the same code location turns prototype into fast mode
Foo.prototype.a; // This kind of access triggers the behavior too
}

// Add more properties (prototype stays in fast mode; map copied)
Foo.prototype.c = function() { ... }

// Map copied again
Foo.prototype.d = function() { ... }

jQuery init phase also has this access pattern:

  • jQuery.fn is set as prototype (it's jQuery.prototype and also init.prototype).
  • jQuery.fn.extend is called repeatedly
  • The property access comes from the line src = target[ name ]; inside jQuery.fn.extend
  • And we also keep adding properties to jQuery.fn as part of jQuery.fn.extend: target[ name ] = ...;

This is a lot of V8 internals (also fixing this on V8 side is on our radar), but looks like the jQuery side fix is simple and also makes sense software-engineering-wise: move "src = target[ name ];" into the branch where it's used. When calling extend with an object which contains functions, we don't go into that branch.

Link to test case

This is not a functional change.

marjakh added a commit to marjakh/jquery that referenced this issue Nov 29, 2018

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 jquery#4245

@mgol mgol added this to the 3.4.0 milestone Nov 29, 2018

marjakh added a commit to marjakh/jquery that referenced this issue Nov 30, 2018

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 jquery#4245

@mgol mgol closed this in #4246 Dec 12, 2018

mgol added a commit that referenced this issue Dec 12, 2018

Core: Tiny efficiency fix to jQuery.extend / jQuery.fn.extend (#4246)
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 the issue for more details).

Fixes gh-4245
Closes gh-4246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.