Opened 11 years ago
Closed 10 years ago
#10290 closed feature (wontfix)
ELEMENT TRAVERSING API
Reported by: | markel | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | 2.0 |
Component: | traversing | Version: | git |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description (last modified by )
http://ejohn.org/blog/element-traversal-api/
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:1 Changed 11 years ago by
comment:3 Changed 11 years ago by
Component: | unfiled → traversing |
---|---|
Priority: | undecided → low |
Resolution: | → wontfix |
Status: | new → closed |
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
Keywords: | 1.8-discuss added |
---|---|
Milestone: | None → 1.8 |
I'm actually reopen this for 1.8 discussion
comment:5 Changed 11 years ago by
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
Description: | modified (diff) |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
comment:7 Changed 11 years ago by
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
More reference material: http://ejohn.org/files/jquery.traverse.js
comment:9 Changed 11 years ago by
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
And why did you removed reference to the post? Now text looks kinda silly.
comment:11 Changed 11 years ago by
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
Status: | reopened → open |
---|
comment:13 Changed 11 years ago by
@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
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:16 Changed 11 years ago by
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
-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
@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
-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
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:24 Changed 11 years ago by
Keywords: | 1.8-discuss removed |
---|---|
Milestone: | 1.8 → None |
Resolution: | → wontfix |
Status: | open → closed |
Vote result was negative for 1.8.
comment:25 Changed 11 years ago by
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
Reopening for possible inclusion in 2.0; Orkel has volunteered.
comment:26 Changed 11 years ago by
Milestone: | None → 2.0 |
---|---|
Status: | reopened → open |
comment:27 Changed 10 years ago by
Type: | enhancement → feature |
---|
Bulk change from enhancement to feature.
comment:28 Changed 10 years ago by
Resolution: | → wontfix |
---|---|
Status: | open → closed |
Pull request -- https://github.com/jquery/jquery/pull/507