Bug Tracker

Opened 11 years ago

Closed 10 years ago

#10290 closed feature (wontfix)


Reported by: markel Owned by:
Priority: low Milestone: 2.0
Component: traversing Version: git
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by Rick Waldron)


Three years past, now all major browser are supporting element traversing api. Maybe it's time to reconsider? In some cases, this properties can noticeable increase iterating speed --

nextAll( http://jsperf.com/element-traversing-nextall-with-obstacles ),

children( http://jsperf.com/element-traversing-children-with-obstacles ),

siblings( http://jsperf.com/element-traversing-siblings-with-obstacles ).

In some cases, speed increase is less noticeable --

nextAll( http://jsperf.com/element-traversing-nextall ),

children( http://jsperf.com/element-traversing-children ),

siblings( http://jsperf.com/element-traversing-siblings ).

Or practically does not exits --

parents( http://jsperf.com/element-traversing-parents-with-obstacles ).

Change History (27)

comment:2 Changed 11 years ago by anonymous

The last perftest does not work on ipad,... Type error thrown

comment:3 Changed 11 years ago by Rick Waldron

Component: unfiledtraversing
Priority: undecidedlow
Resolution: wontfix
Status: newclosed

Three years have passed and we still have to support IE6,7,8 the only difference is that now we're adamantly _not_ writing any more feature code for those browsers.

comment:4 Changed 11 years ago by Rick Waldron

Keywords: 1.8-discuss added
Milestone: None1.8

I'm actually reopen this for 1.8 discussion

comment:5 Changed 11 years ago by markel

If browser are supporting new api - we use it, if it don't - we use the old one. Like it shown in pull request. Like it always has, when people start working with new api's.

comment:6 Changed 11 years ago by Rick Waldron

Description: modified (diff)
Resolution: wontfix
Status: closedreopened

comment:7 Changed 11 years ago by markel

About iPad thing --
It's nothing wrong with pull request, it's because in test i use "$.noConflict" instead of "jQuery.noConflict" test here http://jsperf.com/element-traversing-parents-with-obstacles-2 it's should be fine.

comment:8 Changed 11 years ago by Rick Waldron

comment:9 Changed 11 years ago by anonymous

Rick, in ticket discription all test are fine, except the last one ( and problem exist only on iPad ), without them, you cannot see speed increase, they should be there.

comment:10 Changed 11 years ago by anonymous

And why did you removed reference to the post? Now text looks kinda silly.

comment:11 Changed 11 years ago by Rick Waldron

Not on purpose, sorry... that seems to happen when I post from my mobile... I'll fix it when I get home from NYC tonight

comment:12 Changed 11 years ago by timmywil

Status: reopenedopen

comment:13 Changed 11 years ago by dmethvin

@markel, is there some perf test other than the one in comment 7? It doesn't look like there is any performance benefit.

comment:14 Changed 11 years ago by markel

Yes of cource, test in comment for "parents" method, with it you got nothing, but for others you got something. See http://bugs.jquery.com/ticket/10290#comment:9 and http://bugs.jquery.com/ticket/10290#comment:11

You can see those test in history of ticket description -- http://bugs.jquery.com/ticket/10290?action=diff&version=6

I guess Rick forgot to update the ticket.

comment:15 Changed 11 years ago by dmethvin

Description: modified (diff)

I restored the links.

comment:16 Changed 11 years ago by jaubourg

Description: modified (diff)

+0, I hate to see a new code path... even if perf is twice as good, what is the actual perf gain in real-life apps?

comment:18 Changed 11 years ago by dmethvin

-1, Isolated perfs look good, but the current code is no slouch and traversing isn't common enough to justify the size increase and extra code paths.

comment:19 Changed 11 years ago by markel

@jaubourg its pretty hard or maybe even not possible to distinguish traversing methods perf, from others methods perf (DOM manipulations, event attach, etc.) in real-live code, you always use them together, because if you travers, you got a reason for it, but it does not mean they will not have final impact on your app, that makes it slow.

@dmethvin maybe you meant that traverse method is not bottleneck in most apps? They very common by themselves. You already have extra code paths for IE, this is the exactly the same thing. Current code is not a slouch, but like i mentioned before, if you use traverse methods one after another, they might became slow -- http://jsperf.com/etapi-cumulative-effect

comment:20 Changed 11 years ago by mikesherov

-1, I'd need a real world example showing element traversal being the bottleneck in a particular case to support this. All of the perfs mentioned in this ticket are either isolated or contrived. I'm being earnest when I say show me a real example... perhaps I can be swayed.

comment:21 Changed 11 years ago by markel

jQuery.fn.closest and jQuery.fn.parents is good example btw. Very diffrent code paths, very diffrent speed results, jQuery.fn.closest implementation add a lot more extra bytes and jQuery.fn.parents is no slouch too -- http://jsperf.com/closest-vs-parents

And now, can you find a real-life code example that justifies jQuery.fn.closest existence? Apps that bottlenecking on jQuery.fn.parents?

This example is almost exactly like my Element Traversing API implementation. Even speed results, code paths and amount of bytes is comparable. Same pros, same cons. And yet...

comment:22 Changed 11 years ago by timmywil

-1, fc

comment:23 Changed 11 years ago by Rick Waldron

Description: modified (diff)


comment:24 Changed 11 years ago by dmethvin

Keywords: 1.8-discuss removed
Milestone: 1.8None
Resolution: wontfix
Status: openclosed

Vote result was negative for 1.8.

comment:25 Changed 11 years ago by dmethvin

Resolution: wontfix
Status: closedreopened

Reopening for possible inclusion in 2.0; Orkel has volunteered.

comment:26 Changed 11 years ago by dmethvin

Milestone: None2.0
Status: reopenedopen

comment:27 Changed 10 years ago by dmethvin

Type: enhancementfeature

Bulk change from enhancement to feature.

comment:28 Changed 10 years ago by markelog

Resolution: wontfix
Status: openclosed

Fix was commited to 2.0 beta (65bdfbf), but it was reverted, see #13265.

Note: See TracTickets for help on using tickets.