jQuery: Ticket #14839: jQuery.data first access performance very badhttps://bugs.jquery.com/ticket/14839
<p>
<a class="ext-link" href="http://jsfiddle.net/6DLSw/12/"><span class="icon">​</span>http://jsfiddle.net/6DLSw/12/</a>
2s+ first time, ~50ms later on. The first run is extremely slow.
</p>
<p>
<a class="ext-link" href="http://jsfiddle.net/6DLSw/13/"><span class="icon">​</span>http://jsfiddle.net/6DLSw/13/</a>
~50ms when jQuery.data(el) was already called (note that the key doesn't matter).
</p>
<p>
The bug seems to have started with version 2.0.2 or so, when the jQuery.data was rewritten.
</p>
<p>
Using 1.11.0 on first case takes ~190ms first time, other times it's ~60ms.
</p>
<p>
This is affecting jquery ui performance very badly, e.g. <a class="ext-link" href="http://jsfiddle.net/6DLSw/6/"><span class="icon">​</span>http://jsfiddle.net/6DLSw/6/</a> - first red rectangle drop takes 2s+.
</p>
en-usjQuery//jquery.com/images/logo.gifhttps://bugs.jquery.com/ticket/14839
Trac 1.2.3Rick WaldronSun, 02 Mar 2014 19:14:18 GMTstatus changed; owner sethttps://bugs.jquery.com/ticket/14839#comment:1
https://bugs.jquery.com/ticket/14839#comment:1
<ul>
<li><strong>owner</strong>
set to <em>Rick Waldron</em>
</li>
<li><strong>status</strong>
changed from <em>new</em> to <em>assigned</em>
</li>
</ul>
<p>
The internal design of jQuery.data and jQuery.fn.data will be changing again in a forth coming release.
</p>
TicketdmethvinMon, 03 Mar 2014 15:50:31 GMTpriority, component, milestone changedhttps://bugs.jquery.com/ticket/14839#comment:2
https://bugs.jquery.com/ticket/14839#comment:2
<ul>
<li><strong>priority</strong>
changed from <em>undecided</em> to <em>high</em>
</li>
<li><strong>component</strong>
changed from <em>unfiled</em> to <em>data</em>
</li>
<li><strong>milestone</strong>
changed from <em>None</em> to <em>1.12/2.2</em>
</li>
</ul>
<p>
See also <a class="closed ticket" href="https://bugs.jquery.com/ticket/11570" title="#11570: feature: Move element cache to the element[expando] to avoid cleanup and reduce ... (closed: migrated)">#11570</a> and <a class="ext-link" href="https://github.com/jquery/jquery/pull/1428"><span class="icon">​</span>https://github.com/jquery/jquery/pull/1428</a>
</p>
TicketdmethvinMon, 28 Apr 2014 13:59:32 GMThttps://bugs.jquery.com/ticket/14839#comment:3
https://bugs.jquery.com/ticket/14839#comment:3
<p>
<a class="closed ticket" href="https://bugs.jquery.com/ticket/15059" title="#15059: bug: jQuery 2 $.data slower then jQuery 1 (closed: duplicate)">#15059</a> is a duplicate of this ticket.
</p>
TicketjbedardMon, 28 Apr 2014 16:12:43 GMThttps://bugs.jquery.com/ticket/14839#comment:4
https://bugs.jquery.com/ticket/14839#comment:4
<p>
Sorry for the dupe.
</p>
<p>
Is the internal design change the solution proposed for <a class="closed ticket" href="https://bugs.jquery.com/ticket/11570" title="#11570: feature: Move element cache to the element[expando] to avoid cleanup and reduce ... (closed: migrated)">#11570</a>? Or are there more plans? The pull request for <a class="closed ticket" href="https://bugs.jquery.com/ticket/11570" title="#11570: feature: Move element cache to the element[expando] to avoid cleanup and reduce ... (closed: migrated)">#11570</a> is still using Object.defineProperties, so I assume the data creation will still be very slow...
</p>
TicketdmethvinMon, 28 Apr 2014 16:24:10 GMThttps://bugs.jquery.com/ticket/14839#comment:5
https://bugs.jquery.com/ticket/14839#comment:5
<p>
Yes, the PR is mainly a proof of concept to see if we could attach data directly to DOM elements to avoid doing cleanup and reduce the chances of leaks. I suspect we'll need to abandon the nice encapsulation to get speed back.
</p>
TicketRick WaldronWed, 07 May 2014 21:20:54 GMThttps://bugs.jquery.com/ticket/14839#comment:6
https://bugs.jquery.com/ticket/14839#comment:6
<p>
I'd like to see benchmarks for patterns that reflect real pathological use cases—accessing jQuery data on 60k divs at once isn't realistic.
</p>
<p>
Not using Object.defineProperty means:
</p>
<ul><li>the expando property is visible on all objects with data, for elements this isn't really a big deal, but extant code wants to use <code>var o = {}; $(o).data(...)</code> which means that <code>o</code> will now have an writable, configurable and enumerable property for _both_ user data and jQuery internal data.
</li><li>data properties appearing in for-in, the array returned by Object.keys and <code> jQuery.each</code> iterations
</li><li>data properties appearing in JSON, which can be fixed by falling back to the old toJSON hack
</li><li>...but then you have 1 too many properties on every object, which fails tests. Of course this too can be patched, but then the object that exists as the value of this expando won't match what's returned by .data()
</li></ul>
TicketRick WaldronWed, 07 May 2014 21:48:31 GMThttps://bugs.jquery.com/ticket/14839#comment:7
https://bugs.jquery.com/ticket/14839#comment:7
<blockquote class="citation">
<p>
data properties appearing in JSON, which can be fixed by falling back to the old toJSON hack
</p>
</blockquote>
<p>
This also means breaking:
</p>
<ul><li>jQuery.hasData agrees no data exists even when an empty data obj exists
</li><li>data() returns entire data object with expected properties
</li><li>Retrieve data object from a wrapped JS object (<a class="closed ticket" href="https://bugs.jquery.com/ticket/7524" title="#7524: bug: Calling $.fn.data without arguments on a non-DOMelement in 1.4.4 (closed: fixed)">#7524</a>)
</li><li>removeData does not clear the object
</li><li>Make sure that the right number of properties came through.
</li><li>Data appears as expected.
</li><li>Data is empty.
</li><li>Sanity Check
</li><li>The data didn't change even though the data-* attrs did
</li></ul><p>
I'm confident all of these can be "fixed" but I'm concerned that the resulting code will be a maintenance nightmare.
</p>
TicketjbedardTue, 13 May 2014 07:41:14 GMThttps://bugs.jquery.com/ticket/14839#comment:8
https://bugs.jquery.com/ticket/14839#comment:8
<p>
I'm not sure if the defineProperty/ies is the only or main cause of this, I thought it was (<a class="ext-link" href="http://jsperf.com/v1-v2-data"><span class="icon">​</span>http://jsperf.com/v1-v2-data</a>), but after a quick test replacing it with a plain assignment my use case was still significantly slower then jQuery1 (but this was 1 quick test...). If the defineProp is a big factor could a simple assignment be used for elements and still use defineProp for other objects?
</p>
<p>
My use case is a table where every cell has data. With approx 10k cells (500 rows, 20 columns) jQuery2 loads about 66% slower (~2.5s vs ~1.5s). That extra 1s is mainly Data.key (~700ms) and GC (~300ms).
</p>
<p>
Here's an example (50 rows x 10 columns) that seems to reproduce it: <a class="ext-link" href="http://jsperf.com/jq2-data/5"><span class="icon">​</span>http://jsperf.com/jq2-data/5</a>
</p>
<p>
From profiling the 2 cases it looks like this jsperf reproduces almost the exact same issue I'm having. With jQuery1 the forced relayout is most expensive (~25%), the clone/append/data/GC are all fairly small (2-5% each). With jQuery2 Data.key (~30%) and GC (~25%) skyrocket making the relayout only ~10%.
</p>
<p>
It looks like this might only really effect chrome though? That surprised me...
</p>
TicketjbedardThu, 15 May 2014 08:57:56 GMThttps://bugs.jquery.com/ticket/14839#comment:9
https://bugs.jquery.com/ticket/14839#comment:9
<p>
It looks like Chrome is acting up due to the long data expando property name. I assume the longer name is treated differently or causes the element properties to be stored differently?
</p>
<p>
This seems to fix everything: <a class="ext-link" href="https://github.com/jbedard/jquery/commit/ed3a9887d16b098a7c784cc4920185d9e5c53e2a"><span class="icon">​</span>https://github.com/jbedard/jquery/commit/ed3a9887d16b098a7c784cc4920185d9e5c53e2a</a>
</p>
<p>
That change makes <a class="ext-link" href="http://jsperf.com/jq2-data/5"><span class="icon">​</span>http://jsperf.com/jq2-data/5</a> run 3x faster in chrome. It makes <a class="ext-link" href="http://jsfiddle.net/6DLSw/12/"><span class="icon">​</span>http://jsfiddle.net/6DLSw/12/</a> go from 3000+ to ~500 on the first run. It removes all the GC CPU usage. And it made a test I had locally (basically <a class="ext-link" href="http://jsperf.com/jq2-data/5"><span class="icon">​</span>http://jsperf.com/jq2-data/5</a> called 100x) go from a 470m heap to 27m.
</p>
<p>
Here's an example showing the setting of different sized property names on elements: <a class="ext-link" href="http://jsperf.com/long-name-on-dom"><span class="icon">​</span>http://jsperf.com/long-name-on-dom</a>
In this example FF/IE actually improve more then Chrome so I don't understand why the original bug seems to mainly effect Chrome and not the others. Maybe only Chrome also has the memory issues?
</p>
<p>
Also note that using plain assignment instead of defineProperty/ies (maybe only for elements?) still makes it another 5x faster. Only then does Data.key get reduced enough that it uses less time then the DOM manipulations (like v1) in the jsperf and jsfiddle examples...
</p>
TicketdmethvinThu, 15 May 2014 12:13:59 GMThttps://bugs.jquery.com/ticket/14839#comment:10
https://bugs.jquery.com/ticket/14839#comment:10
<p>
Awesome, @jbedard, thanks for drilling down on this! It seems like we should be able to use just a uid there, the critical thing is that we allow multiple jQueries to work on an element without conflict and <code>jQuery.expando</code> should ensure that.
</p>
<p>
@rwaldron, what do you think?
</p>
TicketjbedardThu, 05 Jun 2014 05:08:32 GMThttps://bugs.jquery.com/ticket/14839#comment:11
https://bugs.jquery.com/ticket/14839#comment:11
<p>
<a class="ext-link" href="https://code.google.com/p/chromium/issues/detail?id=378607"><span class="icon">​</span>https://code.google.com/p/chromium/issues/detail?id=378607</a> might be the cause of this, meaning it would be the '.' from Math.random() causing the issue not the length of the string.
</p>
TicketdmethvinThu, 05 Jun 2014 05:12:05 GMThttps://bugs.jquery.com/ticket/14839#comment:12
https://bugs.jquery.com/ticket/14839#comment:12
<p>
That's interesting. At least the fix would be easy.
</p>
TicketjbedardWed, 10 Sep 2014 20:29:14 GMThttps://bugs.jquery.com/ticket/14839#comment:13
https://bugs.jquery.com/ticket/14839#comment:13
<p>
It looks like Sizzle also puts a property onto DOM elements containing a dash ("sizzle-timestamp")...
</p>
<p>
Any update on this? It would be great to at least remove the "-"s for now to avoid the main chrome issue...
</p>
TicketjbedardWed, 10 Sep 2014 21:41:12 GMThttps://bugs.jquery.com/ticket/14839#comment:14
https://bugs.jquery.com/ticket/14839#comment:14
<p>
The "delete elem[ internalKey ]" in $.cleanData seems to cause the same thing. When combined with <a class="ext-link" href="https://code.google.com/p/v8/issues/detail?id=2073#c26"><span class="icon">​</span>https://code.google.com/p/v8/issues/detail?id=2073#c26</a> this can lead to chrome crashing. In my case I'm deleting a giant grid with &gt;300k nodes, and the delete line cause each node to increase in memory over 10x.
</p>
TicketmarkelogFri, 26 Sep 2014 11:11:32 GMThttps://bugs.jquery.com/ticket/14839#comment:15
https://bugs.jquery.com/ticket/14839#comment:15
<p>
After landing <a class="ext-link" href="https://github.com/jquery/jquery/pull/1662"><span class="icon">​</span>https://github.com/jquery/jquery/pull/1662</a> it seems we improved here, but
</p>
<blockquote class="citation">
<p>
Also note that using plain assignment instead of defineProperty/ies (maybe only for elements?) still makes it another 5x faster
</p>
</blockquote>
<p>
Sounds like using <code>defineProperties</code> is bad idea, as of now that is, should we get back to using plain assignment?
</p>
TicketjbedardFri, 26 Sep 2014 16:05:46 GMThttps://bugs.jquery.com/ticket/14839#comment:16
https://bugs.jquery.com/ticket/14839#comment:16
<p>
Even switching from defineProperties to defineProperty (since we only have one) helps a bit. But really I think something such as <a class="ext-link" href="https://github.com/jbedard/jquery/commit/d44885b3603dd41ebc47ac889bc55a9a0da0fd4f"><span class="icon">​</span>https://github.com/jbedard/jquery/commit/d44885b3603dd41ebc47ac889bc55a9a0da0fd4f</a> would be best.
</p>
Ticketgibson042Fri, 26 Sep 2014 20:43:21 GMThttps://bugs.jquery.com/ticket/14839#comment:17
https://bugs.jquery.com/ticket/14839#comment:17
<p>
Replying to <a class="ticket" href="https://bugs.jquery.com/ticket/14839#comment:16" title="Comment 16">jbedard</a>:
</p>
<blockquote class="citation">
<p>
Even switching from defineProperties to defineProperty (since we only have one) helps a bit. But really I think something such as <a class="ext-link" href="https://github.com/jbedard/jquery/commit/d44885b3603dd41ebc47ac889bc55a9a0da0fd4f"><span class="icon">​</span>https://github.com/jbedard/jquery/commit/d44885b3603dd41ebc47ac889bc55a9a0da0fd4f</a> would be best.
</p>
</blockquote>
<p>
I agree; our current implementation is just trying to be a bit too clever.
</p>
Ticketm_golMon, 20 Oct 2014 23:20:10 GMTstatus changed; resolution sethttps://bugs.jquery.com/ticket/14839#comment:18
https://bugs.jquery.com/ticket/14839#comment:18
<ul>
<li><strong>status</strong>
changed from <em>assigned</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>migrated</em>
</li>
</ul>
<p>
Migrated to <a class="ext-link" href="https://github.com/jquery/jquery/issues/1728"><span class="icon">​</span>https://github.com/jquery/jquery/issues/1728</a>
</p>
TicketJason BedardThu, 11 Dec 2014 22:50:55 GMThttps://bugs.jquery.com/ticket/14839#comment:19
https://bugs.jquery.com/ticket/14839#comment:19
<p>
Data: avoid non-alphanumeric chars in expando properties
</p>
<p>
Ref chromium issue 378607
Ref <a class="closed ticket" href="https://bugs.jquery.com/ticket/14839" title="#14839: bug: jQuery.data first access performance very bad (closed: migrated)">#14839</a>
Closes gh-1662
</p>
<blockquote>
<p>
Changeset: <a class="missing changeset" rel="nofollow" title="No changeset b876e50834f109619b9607b1d63c3869e2f04ffe in the repository">b876e50834f109619b9607b1d63c3869e2f04ffe</a>
</p>
</blockquote>
TicketJason BedardThu, 11 Dec 2014 22:57:09 GMThttps://bugs.jquery.com/ticket/14839#comment:20
https://bugs.jquery.com/ticket/14839#comment:20
<p>
Data: avoid non-alphanumeric chars in expando properties
</p>
<p>
(cherry-picked from <a class="changeset" href="https://bugs.jquery.com/changeset/0cdec797de23555c95a70978f4d9e06f3b041330/" title="Data: avoid non-alphanumeric chars in expando properties
Ref chromium ...">0cdec797de23555c95a70978f4d9e06f3b041330</a>)
</p>
<p>
Ref chromium issue 378607
Ref <a class="closed ticket" href="https://bugs.jquery.com/ticket/14839" title="#14839: bug: jQuery.data first access performance very bad (closed: migrated)">#14839</a>
Closes gh-1662
</p>
<blockquote>
<p>
Changeset: <a class="changeset" href="https://bugs.jquery.com/changeset/9448be717ce7ccc21dccf7e3d8d929252f270a89/" title="Data: avoid non-alphanumeric chars in expando properties
...">9448be717ce7ccc21dccf7e3d8d929252f270a89</a>
</p>
</blockquote>
Ticket