Bug Tracker

Modify

Ticket #5087 (open bug)

Opened 2 years ago

Last modified 2 months ago

Use element.classList if available to speed up class manipulation

Reported by: sylvain Owned by: rwaldron
Priority: low Milestone: 1.next
Component: attributes Version: 1.3.2
Keywords: Cc: rwaldron
Blocking: Blocked by: #5766

Description

Element.classList is a new API introduced in HTML5 for manipulating the classes of an element. The API is documented on  https://developer.mozilla.org/en/DOM/element.classList.

This API is implemented in Firefox 3.6 and WebKit is planning to implement it.

JQuery could detect if this property is available and use it in the {add,had,remove,toggle}Class functions. A 3-90 speedup could be achieved with Firefox according to this benchmark:  https://bug501257.bugzilla.mozilla.org/attachment.cgi?id=395627).

Change History

comment:2 Changed 20 months ago by dmethvin

  • Component changed from unfiled to selector

comment:3 Changed 15 months ago by rwaldron

  • Owner set to rwaldron
  • Status changed from new to assigned

comment:4 Changed 15 months ago by rwaldron

  • Priority changed from minor to low
  • Milestone changed from 1.4 to 1.5

comment:5 Changed 14 months ago by paul.irish

#7378 is a duplicate of this ticket.

comment:6 Changed 14 months ago by paul.irish

  • Blocked by 5766 added

comment:7 Changed 13 months ago by MT

Apparently element.classList is currently implemented in both Firefox (3.6+) and Chrome (at least 8.0.552.237+). Two independent implementations are quite good reason to use this native feature in jQuery to improve performance.

comment:8 follow-up: ↓ 9 Changed 13 months ago by rwaldron

There has been extensive research into the practical application of using classList internally - we're not in any rush to wrap two very different logic scenarios for a small perf boost

comment:9 in reply to: ↑ 8 Changed 13 months ago by MT

Replying to rwaldron: Small boost? Do you have some exact numbers? What do you think about benchmark results mentioned in the initial topic message? Thanks.

comment:10 Changed 13 months ago by rwaldron

@MT I probably shouldn't have used a quantifying adjective.

I'm revisiting the benchmarks I wrote last fall.

comment:11 Changed 13 months ago by rwaldron

When you make classList do what jQuery does (which is what it would have to do if it was used in the implementation), with regard to multiple classes, its not faster:

 http://jsperf.com/classlist-vs-addclass

I'd like to note that when I wrote a 5 class, 100 element test, the classList functions were faster in some cases.

Last edited 13 months ago by rwaldron (previous) (diff)

comment:12 Changed 13 months ago by MT

Replying to rwaldron: Thanks, this is interesting. Nevertheless, adding multiple classes to one element at once is quite rare (since just useless) operation. So, such benchmarks are not enough representative.

At the outside, classList might be used for adding single class, while pure script implementation—for adding multiple classes.

comment:13 follow-up: ↓ 14 Changed 13 months ago by rwaldron

Is it really worth it? To add code that checks for support of classList and checks if the split string has length > 1 then use classList...? Seems like a lot of work

Trust me, I'm the biggest fan of classList and even proposed a new function call classlist() - that was faster in many regards.

comment:14 in reply to: ↑ 13 Changed 12 months ago by MT

Replying to rwaldron:

Is it really worth it? To add code that checks for support of classList and checks if the split string has length > 1 then use classList...? Seems like a lot of work

From practice perspective, I believe that adding single class is used far most often than adding multiple classes at once. So, if using native classList will result in any performance boost with single class case, then why not? ;-) We can even accept small perfomance loss in multiple class adding operation at all (to exclude an additional check for whether one or multiple classes are added) since this is rarely used.

proposed a new function call classlist() - that was faster in many regards.

What is the function you are talking about? Some potential jQuery.classlist()? Can you provide a details?

comment:15 follow-up: ↓ 17 Changed 12 months ago by rwaldron

Have you looked that jQuery UI source? Some examples:

jquery.ui.tooltip.js, L37
addClass("ui-tooltip ui-widget ui-corner-all ui-widget-content")

jquery.ui.tabs.js, L 146
addClass( "ui-tabs-panel ui-widget-content ui-corner-bottom" )

jquery.ui.tabs.js, L 160
this.element.addClass( "ui-tabs ui-widget ui-widget-content ui-corner-all" );

jquery.ui.tabs.js, L 161
this.list.addClass( "ui-tabs-nav ui-helper-reset ui-helper-clearfix ui-widget-header ui-corner-all" );

jquery.ui.tabs.js, L 162
this.lis.addClass( "ui-state-default ui-corner-top" );

jquery.ui.tabs.js, L 163
this.panels.addClass( "ui-tabs-panel ui-widget-content ui-corner-bottom" );

Even more... 
jquery.ui.tabs.js, L 210, 214, 283, 291, 300, 307, 419



You get the idea ;)

As for the classlist() method I had previously been campaigning for:

 https://github.com/rwldrn/jquery-classlist

Last edited 12 months ago by rwaldron (previous) (diff)

comment:16 Changed 12 months ago by paul.irish

There's a pull req for this now:  https://github.com/jquery/jquery/pull/228

comment:17 in reply to: ↑ 15 Changed 12 months ago by MT

Replying to rwaldron: When native feature is available, it is the only true way to do things and is preferred over pure script implementation anyway. Native implemenation is at least much more likely to work correctly than pure-script one.

For example, boolean attribute selector ('INPUT[autofocus]') in jQuery works only in browsers that support native querySelector() (see bug 5637). And this is not because native feature is faster or not at all, but because this is native.

An exception for preferring native implementation is when this is entirely (in any use case) and considerably (by more than, say, 1.5x) slower than pure-script one. (Though, even in that case, native implementation may get faster in future versions of browsers while pure-script one will most likely not.)

As for classList() method you've mentioned, then, in my opinion, this might only return list of element classes. For adding multiple classes as JS-array, just regular addClass() should be used (impossibility to add classes as Array object is actually drawback of current addClass() method).

Thanks.

comment:18 Changed 11 months ago by rwaldron

#8447 is a duplicate of this ticket.

comment:19 Changed 11 months ago by danheberden

  • Keywords pullreq added
  • Milestone set to 1.next

comment:20 Changed 10 months ago by john

  • Status changed from assigned to closed
  • Resolution set to patchwelcome

See my comments in the closed pull request (classList appears to be too slow right now). If classList gets faster, or if a better patch is proposed, we'll happily land it.

comment:21 Changed 10 months ago by john

  • Component changed from selector to attributes

comment:22 Changed 5 months ago by rwaldron

#10196 is a duplicate of this ticket.

comment:24 Changed 5 months ago by rwaldron

  • Status changed from closed to reopened
  • Resolution patchwelcome deleted

I'm going to re-open this for possible 1.8 proposal.

comment:25 Changed 5 months ago by addyosmani

  • Cc rwaldron added

Some quick comments: looking through the thread it would appear that the consensus was that for getting jQuery to do with classList what we currently do with multiple classes it was considered too slow to use. This also applied to the pull request that came through a few months ago.

Are there particular performance tests or implementations we're currently aware of that perform better than previously observed or is the intention of the discussion to seek a better implementation that works around what vendors currently have so that we can support this?

(just curious) :)

comment:26 Changed 5 months ago by rwaldron

I'm just going to revive local branches I have and run perf tests. Thees a few things I want to try this time around

comment:27 Changed 5 months ago by dmethvin

I'd leave the ticket closed unless there's a clear reason to revive it based on new benchmarks for example.

comment:28 Changed 5 months ago by timmywil

  • Keywords pullreq, 1.8-discuss added; pullreq removed
  • Status changed from reopened to open

comment:29 Changed 2 months ago by rwaldron

  • Keywords pullreq, removed

The performance issue of setting multiple classes still exists.

comment:30 Changed 2 months ago by rwaldron

  • Keywords 1.8-discuss removed

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as open
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.