[mochikit] Re: Selector speedup by using John Resig's Sizzle
Arnar, thanks for the update! I'm in no hurry myself, having plenty of other things to focus on. :-) Hope you have a nice trip! Cheers, /Per On Thu, Nov 20, 2008 at 11:55 AM, Arnar Birgisson [EMAIL PROTECTED] wrote: Hi Per, On Tue, Nov 18, 2008 at 23:04, Per Cederberg [EMAIL PROTECTED] wrote: Perhaps the time is right now to just go ahead and merge the new selector branch into the 1.5 trunk? With the appropriate updates from the base Sizzle implementation of course. If Sizzle is considered stable, I'd say it is time to do so. I haven't been able to give any attention to this for the last few weeks as I've been travelling in the States, Italy and the Netherlands -- with various levels of connection -- and will not be able to until December at least. Also, please note that the MochiKit Customizer assumes that each module starts with a call to _deps and ends with a call to _export, so please make sure that the new Selector module also follows that pattern. Could you add a ticket for it? If no-one else takes care of it, then as I said I won't be available until December. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi Per, On Tue, Nov 18, 2008 at 23:04, Per Cederberg [EMAIL PROTECTED] wrote: Perhaps the time is right now to just go ahead and merge the new selector branch into the 1.5 trunk? With the appropriate updates from the base Sizzle implementation of course. If Sizzle is considered stable, I'd say it is time to do so. I haven't been able to give any attention to this for the last few weeks as I've been travelling in the States, Italy and the Netherlands -- with various levels of connection -- and will not be able to until December at least. Also, please note that the MochiKit Customizer assumes that each module starts with a call to _deps and ends with a call to _export, so please make sure that the new Selector module also follows that pattern. Could you add a ticket for it? If no-one else takes care of it, then as I said I won't be available until December. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
I don't follow the Sizzle development very closely, but I seem to remember having seem some check-ins dealing with similar issues. Perhaps the time is right now to just go ahead and merge the new selector branch into the 1.5 trunk? With the appropriate updates from the base Sizzle implementation of course. Also, please note that the MochiKit Customizer assumes that each module starts with a call to _deps and ends with a call to _export, so please make sure that the new Selector module also follows that pattern. Cheers, /Per On Tue, Nov 4, 2008 at 11:36 AM, Arnar Birgisson [EMAIL PROTECTED] wrote: Hello Giulio Cesare, First: wow - your name is awesome :) On Mon, Nov 3, 2008 at 11:56, Giulio Cesare Solaroli [EMAIL PROTECTED] wrote: I would like to ask if the new implementation is currentWindow aware or (like it seems form a quick look at the code) is still accessing directly the document object. I will try to make a few test case to see if I am doing something wrong with my current code or there is really a problem here. I want the new implementation to be currentWindow aware (or rather currentDocument) - and I tried to put the call in the right places. However, I'm not entirely sure that Sizzle handles this correctly. I.e. Sizzle might implicitly make the assumption that one is using window.document in some places. I pointed this out to John and he could not confirm either way, I think he means to have a look at it. In the meantime, test-cases for this would be greatly appreciated. If you find specific problems, please make a track issue and/or submit a patch if you can. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Oh, I forgot to mention that I put the test suite online here: http://ejohn.org/apps/sizzle/test/ and the performance suite is here: http://ejohn.org/apps/sizzle/speed/ Taking a quick peek at IE 6 I see a lot of areas in which small improvements could yield large results (div p, div + p, .class). As it is it's faster than all the other major libraries. DOMAssistant has some tricks which could definitely help here so I'll investigate and report back. --John On Thu, Oct 23, 2008 at 11:37 AM, John Resig [EMAIL PROTECTED] wrote: Hey All - Sizzle is now passing the test suite 100% in IE 6, Firefox 3, and Safari 3.1. There is one failing test in Firefox 3.1b1 and in Opera 9.6 (both are specific browser bugs, and relatively minor, so I'm filing those with the vendors). I've fixed all of the previously-discussed issues in this thread. With compliance in order I want to look back and tackle two things: - Library-specific hook code (it's looking likely that Sizzle might make its way in to jQuery, MochiKit, Dojo, and Prototype). - Speed (the performance in IE can still use some work so I'm looking in to that) --John On Mon, Oct 20, 2008 at 12:13 PM, Arnar Birgisson [EMAIL PROTECTED]wrote: On Mon, Oct 20, 2008 at 17:40, John Resig [EMAIL PROTECTED] wrote: I don't know what other MochiKitters say about including Sizzle.js as a seperate file. Per, Bob? So it seems like the major difference is that your selector method (findChildElements) is able to take an array of results, correct? Yes, correct. This is something we inherited from Prototype, from where the current Selector module was ported. Perhaps we should change the API, I don't know.. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
My guess would be that the speed difference stems from the Sizzle strategy of searching inside-out from the expression. I.e. using the last part of the query first, and then filtering that set using the previous parts. Perhaps other libraries search outside-in? In the case of ul .tocline2 the approach taken by Sizzle seems to fare especially poorly (compared to the others). Of course, this issue would only affect cases where we are using parent-child relations. Cheers, /Per - running IE6 inside CrossOver, so that might slow down my results a bit too On Thu, Oct 23, 2008 at 6:12 PM, John Resig [EMAIL PROTECTED] wrote: Oh, I forgot to mention that I put the test suite online here: http://ejohn.org/apps/sizzle/test/ and the performance suite is here: http://ejohn.org/apps/sizzle/speed/ Taking a quick peek at IE 6 I see a lot of areas in which small improvements could yield large results (div p, div + p, .class). As it is it's faster than all the other major libraries. DOMAssistant has some tricks which could definitely help here so I'll investigate and report back. --John On Thu, Oct 23, 2008 at 11:37 AM, John Resig [EMAIL PROTECTED] wrote: Hey All - Sizzle is now passing the test suite 100% in IE 6, Firefox 3, and Safari 3.1. There is one failing test in Firefox 3.1b1 and in Opera 9.6 (both are specific browser bugs, and relatively minor, so I'm filing those with the vendors). I've fixed all of the previously-discussed issues in this thread. With compliance in order I want to look back and tackle two things: - Library-specific hook code (it's looking likely that Sizzle might make its way in to jQuery, MochiKit, Dojo, and Prototype). - Speed (the performance in IE can still use some work so I'm looking in to that) --John On Mon, Oct 20, 2008 at 12:13 PM, Arnar Birgisson [EMAIL PROTECTED] wrote: On Mon, Oct 20, 2008 at 17:40, John Resig [EMAIL PROTECTED] wrote: I don't know what other MochiKitters say about including Sizzle.js as a seperate file. Per, Bob? So it seems like the major difference is that your selector method (findChildElements) is able to take an array of results, correct? Yes, correct. This is something we inherited from Prototype, from where the current Selector module was ported. Perhaps we should change the API, I don't know.. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
My guess would be that the speed difference stems from the Sizzle strategy of searching inside-out from the expression. I.e. using the last part of the query first, and then filtering that set using the previous parts. Perhaps other libraries search outside-in? Possibly - it definitely depends on the test page. It tends to fare poorly on this particular test page but since it's the de facto standard, not much we can do :-/ In the case of ul .tocline2 the approach taken by Sizzle seems to fare especially poorly (compared to the others). Of course, this issue would only affect cases where we are using parent-child relations. Well, I'd be more interested in watching cases like div p since that's two pure tag searches and would expose performance flaws in depth selectors pretty quickly. ui .tocline2 is probably especially slow due to the fact that all class selectors are slow in IE 6, right now. --John --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi all, On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer p.s. I'm running firefox 3.1pre now with the JIT running, and slickspeed tests are interesting as your version of MochiKit.Selector is reported as faster than Sizzle: most of the response times are 1ms for a total time of 45ms vs 174ms for Sizzle itself. I would guess that some compiled code is being cached. MochiKit's current Selector comes in with a time of 4045ms. I just upgraded to FF 3.1b1 and I'm seeing this strange thing. For me, MochiKit trunk completes in 1893ms, MochiKit w/Sizzle in 54ms but Sizzle itself in ~100ms - no matter if it is run before or after the MochiKit/Sizzle combo. So, MochiKit+Sizzle is almost twice as fast as Sizzle standalone. John, can you think of a good reason for this? cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
That's... odd. Are there any selectors that are noticeably faster? Maybe something is failing? --John On Mon, Oct 20, 2008 at 6:07 AM, Arnar Birgisson [EMAIL PROTECTED] wrote: Hi all, On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer p.s. I'm running firefox 3.1pre now with the JIT running, and slickspeed tests are interesting as your version of MochiKit.Selector is reported as faster than Sizzle: most of the response times are 1ms for a total time of 45ms vs 174ms for Sizzle itself. I would guess that some compiled code is being cached. MochiKit's current Selector comes in with a time of 4045ms. I just upgraded to FF 3.1b1 and I'm seeing this strange thing. For me, MochiKit trunk completes in 1893ms, MochiKit w/Sizzle in 54ms but Sizzle itself in ~100ms - no matter if it is run before or after the MochiKit/Sizzle combo. So, MochiKit+Sizzle is almost twice as fast as Sizzle standalone. John, can you think of a good reason for this? cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi John, On Mon, Oct 20, 2008 at 15:52, John Resig [EMAIL PROTECTED] wrote: That's... odd. Are there any selectors that are noticeably faster? Yes, it seems that nested queries are to blame. By nested queries I mean queries that uses the axis combinator, either the implicit descendant axis (like div p) or an explicit axis combinator such as ~, or +. div ~ p is 2ms on MK+Sizzle vs. 13ms on Sizzle. div p is 2ms on MK+Sizzle vs. 4ms on Sizzle. div p is 1ms vs. 3ms div + p is 1ms vs. 5ms div p a is 1ms vs. 8ms Also, a[href][lang][class] is 1ms vs. 9ms. Maybe something is failing? I don't think so, at least the number of elements returned by each is the same in every test. You can run the test benchmark yourself here: http://www.hvergi.net/arnar/public/sizzle/speed/ cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Just to clarify: Are you turning on JIT in 3.1? Do you have a diff of any change(s) that you've made to your copy of Sizzle? --John On Mon, Oct 20, 2008 at 10:05 AM, Arnar Birgisson [EMAIL PROTECTED] wrote: Hi John, On Mon, Oct 20, 2008 at 15:52, John Resig [EMAIL PROTECTED] wrote: That's... odd. Are there any selectors that are noticeably faster? Yes, it seems that nested queries are to blame. By nested queries I mean queries that uses the axis combinator, either the implicit descendant axis (like div p) or an explicit axis combinator such as ~, or +. div ~ p is 2ms on MK+Sizzle vs. 13ms on Sizzle. div p is 2ms on MK+Sizzle vs. 4ms on Sizzle. div p is 1ms vs. 3ms div + p is 1ms vs. 5ms div p a is 1ms vs. 8ms Also, a[href][lang][class] is 1ms vs. 9ms. Maybe something is failing? I don't think so, at least the number of elements returned by each is the same in every test. You can run the test benchmark yourself here: http://www.hvergi.net/arnar/public/sizzle/speed/ cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Of course, FF 3.1 includes querySelectorAll:
http://ejohn.org/blog/queryselectorall-in-firefox-31/
And in fact, there is a slight bug in Sizzle here, causing it to not
use that version when not sending in an explicit 2:nd argument:
Sizzle(..., document)
The problem is here:
if ( document.querySelectorAll ) (function(){
var oldSizzle = Sizzle;
Sizzle = function(query, context, extra){
if ( context === document ) {
try {
return makeArray(context.querySelectorAll(query));
} catch(e){}
}
return oldSizzle(query, context, extra);
};
Sizzle.find = oldSizzle.find;
Sizzle.filter = oldSizzle.filter;
})();
Cheers,
/Per
On Mon, Oct 20, 2008 at 4:05 PM, Arnar Birgisson [EMAIL PROTECTED] wrote:
Hi John,
On Mon, Oct 20, 2008 at 15:52, John Resig [EMAIL PROTECTED] wrote:
That's... odd. Are there any selectors that are noticeably faster?
Yes, it seems that nested queries are to blame. By nested queries I
mean queries that uses the axis combinator, either the implicit
descendant axis (like div p) or an explicit axis combinator such
as ~, or +.
div ~ p is 2ms on MK+Sizzle vs. 13ms on Sizzle.
div p is 2ms on MK+Sizzle vs. 4ms on Sizzle.
div p is 1ms vs. 3ms
div + p is 1ms vs. 5ms
div p a is 1ms vs. 8ms
Also, a[href][lang][class] is 1ms vs. 9ms.
Maybe something is failing?
I don't think so, at least the number of elements returned by each is
the same in every test.
You can run the test benchmark yourself here:
http://www.hvergi.net/arnar/public/sizzle/speed/
cheers,
Arnar
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Wow, thanks for catching that. That's amusing that it was still so
fast without using querySelectorAll, in that case. Just committed the
fix:
http://github.com/jeresig/sizzle/commit/6239a25918f8fd7d56fc97c22815418833a64e00
--John
On Mon, Oct 20, 2008 at 10:47 AM, Per Cederberg [EMAIL PROTECTED] wrote:
Of course, FF 3.1 includes querySelectorAll:
http://ejohn.org/blog/queryselectorall-in-firefox-31/
And in fact, there is a slight bug in Sizzle here, causing it to not
use that version when not sending in an explicit 2:nd argument:
Sizzle(..., document)
The problem is here:
if ( document.querySelectorAll ) (function(){
var oldSizzle = Sizzle;
Sizzle = function(query, context, extra){
if ( context === document ) {
try {
return makeArray(context.querySelectorAll(query));
} catch(e){}
}
return oldSizzle(query, context, extra);
};
Sizzle.find = oldSizzle.find;
Sizzle.filter = oldSizzle.filter;
})();
Cheers,
/Per
On Mon, Oct 20, 2008 at 4:05 PM, Arnar Birgisson [EMAIL PROTECTED] wrote:
Hi John,
On Mon, Oct 20, 2008 at 15:52, John Resig [EMAIL PROTECTED] wrote:
That's... odd. Are there any selectors that are noticeably faster?
Yes, it seems that nested queries are to blame. By nested queries I
mean queries that uses the axis combinator, either the implicit
descendant axis (like div p) or an explicit axis combinator such
as ~, or +.
div ~ p is 2ms on MK+Sizzle vs. 13ms on Sizzle.
div p is 2ms on MK+Sizzle vs. 4ms on Sizzle.
div p is 1ms vs. 3ms
div + p is 1ms vs. 5ms
div p a is 1ms vs. 8ms
Also, a[href][lang][class] is 1ms vs. 9ms.
Maybe something is failing?
I don't think so, at least the number of elements returned by each is
the same in every test.
You can run the test benchmark yourself here:
http://www.hvergi.net/arnar/public/sizzle/speed/
cheers,
Arnar
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Excellent list - I just integrated virtually all of your points:
http://github.com/jeresig/sizzle/commit/93e33dc2a41e2b0aa0e1e1c66368f5d224da80e1
The exception is :visible and :hidden - which should be handled by the
host library.
Also, I wrapped the entireity of Sizzle withing a (function(){ ...
})() which means that no global variables are exposed (save for
Sizzle).
What specific code do you use to hook Sizzle in to your engine? What
I'll probably do is just hook it directly in to the right spot (for
example, overwrite jQuery.find, in the case of jQuery) rather than
introduce a new global variable.
--John
On Mon, Oct 20, 2008 at 10:51 AM, Arnar Birgisson [EMAIL PROTECTED] wrote:
Hi again,
On Mon, Oct 20, 2008 at 16:43, John Resig [EMAIL PROTECTED] wrote:
Just to clarify: Are you turning on JIT in 3.1?
JIT is off.
Do you have a diff of any change(s) that you've made to your copy of Sizzle?
Yup. http://www.hvergi.net/arnar/public/sizzle/diff.txt
cheers,
Arnar
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi John,
On Mon, Oct 20, 2008 at 17:19, John Resig [EMAIL PROTECTED] wrote:
Excellent list - I just integrated virtually all of your points:
http://github.com/jeresig/sizzle/commit/93e33dc2a41e2b0aa0e1e1c66368f5d224da80e1
Excellent. Thanks!
The exception is :visible and :hidden - which should be handled by the
host library.
I agree.
Also, I wrapped the entireity of Sizzle withing a (function(){ ...
})() which means that no global variables are exposed (save for
Sizzle).
What specific code do you use to hook Sizzle in to your engine? What
I'll probably do is just hook it directly in to the right spot (for
example, overwrite jQuery.find, in the case of jQuery) rather than
introduce a new global variable.
Actually, I copied the contents of Sizzle into Selector.js which is
part of MochiKit [1]. Integrating is then just a matter of calling
Sizzle(...) in the correct place in the Selector API. Our plan is to
completely remove the old Selector implementation, i.e. using Sizzle
won't be optional like it looks like your plan for jQuery is.
[1]
http://trac.mochikit.com/browser/mochikit/branches/selector_sizzle/MochiKit/Selector.js
I don't know what other MochiKitters say about including Sizzle.js as
a seperate file. Per, Bob?
cheers,
Arnar
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi again, On Mon, Oct 20, 2008 at 17:01, John Resig [EMAIL PROTECTED] wrote: Wow, thanks for catching that. That's amusing that it was still so fast without using querySelectorAll, in that case. Just committed the fix: http://github.com/jeresig/sizzle/commit/6239a25918f8fd7d56fc97c22815418833a64e00 Well, that makes a big difference :) Now both MK+Sizzle and Sizzle standalone do the benchmark in 53 ms. John, this is an interesting testament to your implementation: On FF 3.1b1 with jit turned OFF the Sizzle selector code (i.e not using querySelectorAll) completes in 55ms. With jit turned ON *and* using the querySelectorAll - it improves only a tiny bit to 53ms :) cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Actually, I copied the contents of Sizzle into Selector.js which is part of MochiKit [1]. Integrating is then just a matter of calling Sizzle(...) in the correct place in the Selector API. Our plan is to completely remove the old Selector implementation, i.e. using Sizzle won't be optional like it looks like your plan for jQuery is. It's only going to be optional during this development process - I'm planning on integrating it (and making it mandatory) in the upcoming jQuery 1.3 release. [1] http://trac.mochikit.com/browser/mochikit/branches/selector_sizzle/MochiKit/Selector.js I don't know what other MochiKitters say about including Sizzle.js as a seperate file. Per, Bob? So it seems like the major difference is that your selector method (findChildElements) is able to take an array of results, correct? --John --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Wed, Oct 15, 2008 at 13:37, Per Cederberg [EMAIL PROTECTED] wrote:
On Wed, Oct 15, 2008 at 1:01 PM, Arnar Birgisson [EMAIL PROTECTED] wrote:
Done. Yup, the number of nodes is not correct (or at least as correct
as th other frameworks). Performance takes a minor hit, mainly because
it is so bad to begin with :)
Ah, you mean now correct... :-)
Heh, yes - I do :) I think I left my typing fu in Iceland.
Disturbing that this fix actually affects performance in a noticable
way. It really shouldn't, except when more than ~100 elements are
matched. Perhaps there are some obvious improvements to be made in my
uniq() implementation:
var uniq = function(arr) {
var res = [];
for (var i = 0; i arr.length; i++) {
if (MochiKit.Base.findIdentical(res, arr[i]) 0) {
res.push(arr[i]);
}
}
return res;
};
I think the above should be O(n^2). Not optimal, but fixing the root
cause means rewriting the module altogether. :-(
We should definitely not invest in a big rewrite at this point. As for
the uniq, this is probably as good as it gets unless we can assume
that identical items are always consecutive. If they are, there's the
obvious O(n) method of course.
Many thanks for your help, Arnar!
You are the one that deserves thanks for your efforts :)
cheers,
Arnar
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Wed, Oct 15, 2008 at 1:01 PM, Arnar Birgisson [EMAIL PROTECTED] wrote:
Done. Yup, the number of nodes is not correct (or at least as correct
as th other frameworks). Performance takes a minor hit, mainly because
it is so bad to begin with :)
Ah, you mean now correct... :-)
Disturbing that this fix actually affects performance in a noticable
way. It really shouldn't, except when more than ~100 elements are
matched. Perhaps there are some obvious improvements to be made in my
uniq() implementation:
var uniq = function(arr) {
var res = [];
for (var i = 0; i arr.length; i++) {
if (MochiKit.Base.findIdentical(res, arr[i]) 0) {
res.push(arr[i]);
}
}
return res;
};
I think the above should be O(n^2). Not optimal, but fixing the root
cause means rewriting the module altogether. :-(
Me too. The current implementation is quite bad and it is my fault for
not updating it since the early Prototype port. Sizzle is quite
simple, there are no specific tricks to make it fast, but the design
is nice.
Yes, it is much clearer. I have a few opinions about it of course, but
I think I'll fork the Sizzle project on github to fix those when/if I
have time.
Many thanks for your help, Arnar!
Cheers,
/Per
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Wed, Oct 15, 2008 at 12:46, Per Cederberg [EMAIL PROTECTED] wrote: I think I fixed the MochiKit.Selector issue with div ~ p in r1429 by adding a uniqueness filter on the results from MochiKit.Selector.findChildElements. Unless the number of returned results is very high it should not degrade performance much. And naturally, this should also fix the issue for div p. Arnar, could you add a new trunk MochiKit to your speed comparison page? Done. Yup, the number of nodes is not correct (or at least as correct as th other frameworks). Performance takes a minor hit, mainly because it is so bad to begin with :) Otherwise, I look forward to replacing the current implementation that is based on eval usage, poor query parsing and slow set handling. Not much fun to debug at all, actually... Me too. The current implementation is quite bad and it is my fault for not updating it since the early Prototype port. Sizzle is quite simple, there are no specific tricks to make it fast, but the design is nice. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
I think I fixed the MochiKit.Selector issue with div ~ p in r1429 by adding a uniqueness filter on the results from MochiKit.Selector.findChildElements. Unless the number of returned results is very high it should not degrade performance much. And naturally, this should also fix the issue for div p. Arnar, could you add a new trunk MochiKit to your speed comparison page? Otherwise, I look forward to replacing the current implementation that is based on eval usage, poor query parsing and slow set handling. Not much fun to debug at all, actually... Cheers, /Per On Wed, Oct 15, 2008 at 10:21 AM, Arnar Birgisson [EMAIL PROTECTED] wrote: Hi Per, On Wed, Oct 15, 2008 at 10:15, Per Cederberg [EMAIL PROTECTED] wrote: On Wed, Oct 8, 2008 at 9:28 PM, Arnar Birgisson [EMAIL PROTECTED] wrote: div p MK trunk returns 142 elements while others return 140 (both numbers are reported incorrect though). The *set* of elements is the same, but MK repeats some elements, which is of course not correct. This seems to happen in the situation where one has divdivp.../p/div/div. If one forgets about performance, this is trivial to fix in MK - but a reasonably performing one might be more tricky. I'll let this one pass. It is slightly incorrect, but can be worked-around by users until 1.5 comes along. I agree. div ~ p MK returns 4120 (!) elements here where others return 183 (both numbers are again considered wrong by SlickSpeed). This is clearly a bug I'll try to fix this. Filed as http://trac.mochikit.com/ticket/321 Thanks, let me know if you have problems. I'm sorry I don't have time to help more these days, I have a few deadlines and a conference coming up in the next two weeks. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Whatever you prefer Bob. But users who want to contribute by means of codding, reporting/fixing bugs, suggesting new ideas or anything should never been restricted anyway because of those few spammers... - Amit Mendapara On Oct 13, 8:19 pm, Bob Ippolito [EMAIL PROTECTED] wrote: Well, the login database is outside of trac since we're using basic auth to login and they are the same credentials that give svn commit access. Disabling anonymous commenting is something that I did because I couldn't be bothered to implement a better spam filter or maintain it. I'm not really sold on launchpad, I think bzr would be too much of a barrier to entry for many people. I would certainly consider moving to google code though, because that would be easy enough. All of our other open source projects are there these days. People that want to use distributed vcs to keep a local branch can do so easily enough with a central svn repo. On Mon, Oct 13, 2008 at 6:47 AM, Amit Mendapara [EMAIL PROTECTED] wrote: The version of trac is okay. Seehttp://trac.edgewall.org/wiki/TracPermissions, you can easily prevent those spams. You can see how TurboGears trac is configured... You can also think about moving MochiKit to Launchpad.net. It's really a good platform to host OpenSource projects with distributed vcs, bug tracking, blueprints and more. Launchpad team has already created a project for MochiKit (so that no one then MochiKit team can claim the ownership, you can contact Launchpad team to get ownership rights). - Amit Mendapara On Oct 13, 6:24 pm, Per Cederberg [EMAIL PROTECTED] wrote: On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara [EMAIL PROTECTED] wrote: Once, I filed a bug report on the trac (related to Sortables), but I was unable to change/comment it later. That's why I never submitted again. Yes, this is very unfortunate. I used to have the same problem, so I hear you loud and clear. The problem is the amount of spam that we'd otherwise receive in bug reports. Don't know if there is some newer version of Trac that fixes this that we could install on the server. Or if there is some other solution that would work. Until that happens, emailing to this list or directly to the bug owner should work. Sorry about that. Cheers, /Per --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
I have seen many problems with MochiKit.Selector while testing MochiKit.Query module. As `Per Cederberg` is preparing for 1.4 release, I think MochiKit.Selector should not be included in 1.4, but let we get something really useful with Sizzle which is going to be integrated in MochiKit (hopefully MochiKit 1.5)... - Amit Mendapara --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Mon, Oct 13, 2008 at 14:43, Amit Mendapara [EMAIL PROTECTED] wrote: I have seen many problems with MochiKit.Selector while testing MochiKit.Query module. You would be helping if you submitted these as bug-reports. Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
I'd appreciate bug reports for the MochiKit.Selector module in Trac or here on the list. I've got 1-2 previously here in this thread that I intend to have a look at soon. Either way, I think we are beyond removing MochiKit.Selector entirely for 1.4. I'll update the docs to point out that it is an *experimental* module that is subject to change. Also, I'll add specific notes for those selectors that won't be compatible with the new Sizzle-powered version. Cheers, /Per On Mon, Oct 13, 2008 at 2:43 PM, Amit Mendapara [EMAIL PROTECTED] wrote: I have seen many problems with MochiKit.Selector while testing MochiKit.Query module. As `Per Cederberg` is preparing for 1.4 release, I think MochiKit.Selector should not be included in 1.4, but let we get something really useful with Sizzle which is going to be integrated in MochiKit (hopefully MochiKit 1.5)... - Amit Mendapara --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Mon, Oct 13, 2008 at 14:52, Per Cederberg [EMAIL PROTECTED] wrote: Also, I'll add specific notes for those selectors that won't be compatible with the new Sizzle-powered version. Assuming the bug in Sizzle that causes [attribute] to fail will be fixed, the only ones that will not work are :root and :*-of-type. I think we are decided to Sizzle and these are both rare and probably take an effort to add to Sizzle. -- so, you could go further than adding a notice and just deprecate them right away. Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Oct 13, 5:52 pm, Per Cederberg [EMAIL PROTECTED] wrote: I'd appreciate bug reports for the MochiKit.Selector module in Trac or here on the list. I've got 1-2 previously here in this thread that I intend to have a look at soon. Once, I filed a bug report on the trac (related to Sortables), but I was unable to change/comment it later. That's why I never submitted again. Anyway, I will prepare one on MochiKit.Selector this night and will post here in this thread instead... - Amit Mendapara --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara [EMAIL PROTECTED] wrote: Once, I filed a bug report on the trac (related to Sortables), but I was unable to change/comment it later. That's why I never submitted again. Yes, this is very unfortunate. I used to have the same problem, so I hear you loud and clear. The problem is the amount of spam that we'd otherwise receive in bug reports. Don't know if there is some newer version of Trac that fixes this that we could install on the server. Or if there is some other solution that would work. Until that happens, emailing to this list or directly to the bug owner should work. Sorry about that. Cheers, /Per --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Mon, Oct 13, 2008 at 15:24, Per Cederberg [EMAIL PROTECTED] wrote: On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara [EMAIL PROTECTED] wrote: Once, I filed a bug report on the trac (related to Sortables), but I was unable to change/comment it later. That's why I never submitted again. Yes, this is very unfortunate. I used to have the same problem, so I hear you loud and clear. The problem is the amount of spam that we'd otherwise receive in bug reports. Don't know if there is some newer version of Trac that fixes this that we could install on the server. Or if there is some other solution that would work. Until that happens, emailing to this list or directly to the bug owner should work. Sorry about that. Do we have a procedure for adding user accounts to Trac, other than simply Ask Bob? cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
The version of trac is okay. See http://trac.edgewall.org/wiki/TracPermissions, you can easily prevent those spams. You can see how TurboGears trac is configured... You can also think about moving MochiKit to Launchpad.net. It's really a good platform to host OpenSource projects with distributed vcs, bug tracking, blueprints and more. Launchpad team has already created a project for MochiKit (so that no one then MochiKit team can claim the ownership, you can contact Launchpad team to get ownership rights). - Amit Mendapara On Oct 13, 6:24 pm, Per Cederberg [EMAIL PROTECTED] wrote: On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara [EMAIL PROTECTED] wrote: Once, I filed a bug report on the trac (related to Sortables), but I was unable to change/comment it later. That's why I never submitted again. Yes, this is very unfortunate. I used to have the same problem, so I hear you loud and clear. The problem is the amount of spam that we'd otherwise receive in bug reports. Don't know if there is some newer version of Trac that fixes this that we could install on the server. Or if there is some other solution that would work. Until that happens, emailing to this list or directly to the bug owner should work. Sorry about that. Cheers, /Per --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Well, the login database is outside of trac since we're using basic auth to login and they are the same credentials that give svn commit access. Disabling anonymous commenting is something that I did because I couldn't be bothered to implement a better spam filter or maintain it. I'm not really sold on launchpad, I think bzr would be too much of a barrier to entry for many people. I would certainly consider moving to google code though, because that would be easy enough. All of our other open source projects are there these days. People that want to use distributed vcs to keep a local branch can do so easily enough with a central svn repo. On Mon, Oct 13, 2008 at 6:47 AM, Amit Mendapara [EMAIL PROTECTED] wrote: The version of trac is okay. See http://trac.edgewall.org/wiki/TracPermissions, you can easily prevent those spams. You can see how TurboGears trac is configured... You can also think about moving MochiKit to Launchpad.net. It's really a good platform to host OpenSource projects with distributed vcs, bug tracking, blueprints and more. Launchpad team has already created a project for MochiKit (so that no one then MochiKit team can claim the ownership, you can contact Launchpad team to get ownership rights). - Amit Mendapara On Oct 13, 6:24 pm, Per Cederberg [EMAIL PROTECTED] wrote: On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara [EMAIL PROTECTED] wrote: Once, I filed a bug report on the trac (related to Sortables), but I was unable to change/comment it later. That's why I never submitted again. Yes, this is very unfortunate. I used to have the same problem, so I hear you loud and clear. The problem is the amount of spam that we'd otherwise receive in bug reports. Don't know if there is some newer version of Trac that fixes this that we could install on the server. Or if there is some other solution that would work. Until that happens, emailing to this list or directly to the bug owner should work. Sorry about that. Cheers, /Per --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Mon, Oct 13, 2008 at 17:19, Bob Ippolito [EMAIL PROTECTED] wrote: I'm not really sold on launchpad, I think bzr would be too much of a barrier to entry for many people. I would certainly consider moving to google code though, because that would be easy enough. All of our other open source projects are there these days. People that want to use distributed vcs to keep a local branch can do so easily enough with a central svn repo. I want to support this, i.e. if the plan is to abandon Trac - Google Code is much more fitting than Launchpad. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi Per,
On Wed, Oct 8, 2008 at 15:49, Per Cederberg [EMAIL PROTECTED] wrote:
This is great work! But since I just took on my code-review-glasses,
here comes a bunch of random comments:
1. The test result from p[lang!=is-IS] has been modified. The
MochiKit.Selector implementation was recently fixed just here,
changing the semantics of all attribute operators to imply attribute
existance. One can argue either way, I guess, but I kind of liked the
existing semantics here.
Ah, ok. I just thought the test was buggy. != is not actually defined
by CSS3, so I went by the jQuery definition which is this:
Matches elements that either don't have the specified attribute or do
have the specified attribute but not with a certain value.
I.e. it explicitly says it matches elements w/o that attribute. Of
course this is no standard, if you think we should use the other
semantics I don't mind.
2. From your speed comparison page for the various framework
implementations, I noded a number of issues in IE 6 (have not tested
IE 7 yet). Sizzle returns errors in 4 of the tests there. In the
README it also says: It's a work in progress! Not ready for use yet!
Yes, I'm actually not sure what the status on Sizzle development is -
but John has at least not updated the github version since posting it
originally. Unfortunately I don't have access to IE6 or time to set up
a virtual machine - but if you try debugging it I'd be happy to help
with the little insight I gained last night.
3. Also on the speed page, I noted that the trunk MochiKit.Selector is
reported to throw errors on a bunch of tests. And there are also a few
places where it returns a different number of results than the others.
This is problematic, since it implies that the exising
MochiKit.Selector is not only slow but also incorrect. I have no great
desire for fixing more issues there, but neither am I a fan of pushing
new (and also not finished) stuff immediately out into a release.
Finally, I'm really trying to push for a release real-soon-now (see my
other mails on the list)...
Yes. If the benchmark tests are to be trusted, that simply means that
the trunk version (which should behave like Prototype did, although
they may have changed the selector engine in Prototype now) is
incorrect. I'll look into what tests are failing and what the
standards have to say about them. If it really is incorrect, all the
more reason to move to a new implementation.
I would also vote against pushing this to a release as it stands now.
4. Have you investigated all the cases where the frameworks differ in
the number of returned elements? Just so that we can be reasonably
sure that the new implementation is indeed correct. Also... I seem to
get 1 result for nearly all tests on MochiKit.Selector w/ Sizzle when
running in Safari 3. Could it be that querySelectorAll returns a
NodeList, not an Array?
I will have a look at the tests and confirm what is the correct
outcome. Safari was also failing on several unit tests for me, I will
also look at that tonight.
5. I agree that the global namespace pollution is an issue. If only
the Sizzle namespace was used in the code, we could easily refactor it
to use MochiKit.Selector.Sizzle or similar to further avoid any
issues.
Ok, I'll start work on that too. I think I would move everything
inside Sizzle (and submit a patch to John) and then move Sizzle into
MochiKit.Selector. It should be easy, just a little legwork.
6. There seems to be an unnecessary whitespace change in MochiKit/MochiKit.js.
There are? The only change I can see is the reording of submodules -
since Selector now depends on Style I had to switch their order. Or do
you mean packed/MochiKit/MochiKit.js?
7. The MochiKit.Selector.findDocElements function is not is the
current API docs. So why do we even export it? Or is that omission by
mistake? (This is a trunk issue)
That is probably a mistake, it should definitely be in the docs. I
will fix this in the trunk.
8. The Sizzle result cache seems to break if people start manipulating
the returned arrays (using shift, pop or similar). Basically this:
return cache doCache ?
(cache[ selector ] = results) :
results;
should become:
if (cache doCache) {
cache[selector] = results.slice(0);
}
return results;
Ok, I'll have a look. Otherwise this is a comment for John I guess.
9. Note that when testing $$ in Safari, you are really only using the
built-in support for document.querySelectorAll (unless that function
throws an error). So adding or modifying functionality requires that
the query is checked before using document.querySelectorAll.
Right, I'm not sure what is best to do here. Should we rely on
querySelectorAll where it is available - simply to have the best
performance, or is identical semantics on all browsers more important?
For the latter there are two ways, don't use querySelectorAll (and
take the performance penalty) or strip out the
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hey Everyone -
Ah, ok. I just thought the test was buggy. != is not actually defined
by CSS3, so I went by the jQuery definition which is this:
Matches elements that either don't have the specified attribute or do
have the specified attribute but not with a certain value.
I.e. it explicitly says it matches elements w/o that attribute. Of
course this is no standard, if you think we should use the other
semantics I don't mind.
Obviously we can discuss this some more but my interpretation of [attr!
=value] was that it was equivalent to :not([attr=value]).
Yes, I'm actually not sure what the status on Sizzle development is -
but John has at least not updated the github version since posting it
originally. Unfortunately I don't have access to IE6 or time to set up
a virtual machine - but if you try debugging it I'd be happy to help
with the little insight I gained last night.
I got bogged down here recently but I'm starting back up (since I want
to be able to land this for jQuery 1.3 this month). What sort of
timeline are you guys on for your 1.4 release?
I would also vote against pushing this to a release as it stands now.
Naturally - IE support is still a minefield.
I will have a look at the tests and confirm what is the correct
outcome. Safari was also failing on several unit tests for me, I will
also look at that tonight.
Let me know if the NodeList/Array mis-match exists, I can look in to
that.
Ok, I'll start work on that too. I think I would move everything
inside Sizzle (and submit a patch to John) and then move Sizzle into
MochiKit.Selector. It should be easy, just a little legwork.
I was planning on just encapsulating all the functionality and
selectively exposing it where needed, specifically:
(function(){
...
var Sizzle = this.Sizzle = function(){
...
};
})();
No need to namespace the random state variables that are used
internally.
8. The Sizzle result cache seems to break if people start manipulating
the returned arrays (using shift, pop or similar). Basically this:
return cache doCache ?
(cache[ selector ] = results) :
results;
should become:
if (cache doCache) {
cache[selector] = results.slice(0);
}
return results;
Ok, I'll have a look. Otherwise this is a comment for John I guess.
That seems like a reasonable change.
Right, I'm not sure what is best to do here. Should we rely on
querySelectorAll where it is available - simply to have the best
performance, or is identical semantics on all browsers more important?
For the latter there are two ways, don't use querySelectorAll (and
take the performance penalty) or strip out the features not supported
by it (and take the feature penalty).
Right now there is no feature penalty to using querySelectorAll.
Sizzle just falls back to the old selector code, if qSA doesn't know
how to handle a selector. If something isn't working correctly in this
regard then it should definitely be fixed.
This is another comment for John I guess. I don't necessarily agree
with you though, its function was relatively clear after pasting it
intohttp://regexpal.com/and playing with it for a minute.
I could attempt to explain it a little bit more with comments, I
guess. I'll see what I can do.
Right, there are several bugs of this kind in Sizzle already.
I'll look in to the one mentioned here.
Thanks for the review guys, I appreciate it!
--John
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi John,
Glad to see you are following our discussion. Btw. are you on the
MochiKit mailinglist, or should I keep cc-ing you explicitly when
there are issues for you?
On Wed, Oct 8, 2008 at 16:41, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:
Obviously we can discuss this some more but my interpretation of [attr!
=value] was that it was equivalent to :not([attr=value]).
My intuition is to agree with this.
I got bogged down here recently but I'm starting back up (since I want
to be able to land this for jQuery 1.3 this month). What sort of
timeline are you guys on for your 1.4 release?
The current suggestion is to go into a two-week feature freeze asap
(so in they very next days I guess) and then release 1.4. The
Sizzle-based Selector will most likely not be a part of that, but I
think the intention is to release 1.5 pretty soon.
Right, I'm not sure what is best to do here. Should we rely on
querySelectorAll where it is available - simply to have the best
performance, or is identical semantics on all browsers more important?
For the latter there are two ways, don't use querySelectorAll (and
take the performance penalty) or strip out the features not supported
by it (and take the feature penalty).
Right now there is no feature penalty to using querySelectorAll.
Sizzle just falls back to the old selector code, if qSA doesn't know
how to handle a selector. If something isn't working correctly in this
regard then it should definitely be fixed.
Ah ok, I will have a look at Safari tonight and figure out what's failing.
Right, there are several bugs of this kind in Sizzle already.
I'll look in to the one mentioned here.
I'll send you a patch with a few corrections (such as function (elem,
i, match) { .. m[i] .. // should be match[i] }) and stuff that I
noticed. Also see the bug about not removing processed parts from the
string which I sent you yesterday.
cheers,
Arnar
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
MochiKit group.
To post to this group, send email to mochikit@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
A little status report. On Wed, Oct 8, 2008 at 18:07, Per Cederberg [EMAIL PROTECTED] wrote: What I was trying to say is that if querySelectorAll returns a result that is inconsistent with the JS implementation, then we're in trouble. If it throws an error (as it presumably does when using non-standard stuff), we already fallback to the JS-implementation, so that should work. Ah yes. Right, the fallback seems to work. The problem on Safari was indeed that it returns a NodeList object but is treated as an array. I simply added a call to convert it to an array. As for the incorrectness in the current MochiKit.Selector module in trunk, these are the tests in the SlickSpeed benchmark that fail or return an incorrect number of elements: div p MK trunk returns 142 elements while others return 140 (both numbers are reported incorrect though). The *set* of elements is the same, but MK repeats some elements, which is of course not correct. This seems to happen in the situation where one has divdivp.../p/div/div. If one forgets about performance, this is trivial to fix in MK - but a reasonably performing one might be more tricky. div ~ p MK returns 4120 (!) elements here where others return 183 (both numbers are again considered wrong by SlickSpeed). This is clearly a bug div, p, a div.example, div.note This fails in MK trunk, and rightly so, as it does not support multiple selectors in one string. Multiple selectors should be passed by using an array argument to findDocElements. h1[id]:contains(Selectors) This fails, as :contains(..) is not supported. p:contains(selectors) This returns an invalid number of elements (presumably all p elements) since :contains is not selected. Prototype has the same behavior p:nth-child(2n) This fails as 2n is not recognized, should be easy to fix (just interpret as 2n+0). p:nth-child(n) Fails, and frankly I don't understand the reasoning here (what does this mean?). Should be interpreted as (1n+0) I guess. p:only-child returns wrong number of results, but then again - so do all the others. Perhaps SlickSpeed is incorrect (the empty string) Fails with an error, which I think is better than returning all elements as some libraries do. Could be handled as a special case but I don't see the need. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi all, On Tue, Oct 7, 2008 at 06:21, JasonBunting [EMAIL PROTECTED] wrote: Any update on this? According to slide 92 of John Resig's recent JavaScript Library Overview slides from Ajax Experience '08 (http://www.slideshare.net/jeresig/javascript-library-overview- presentation/), this is being integrated into MochiKit. Sorry, a sudden onslaught of thesis work has been keeping me busy. I was just thinking about this last night though. I can upload a patch for review tonight. There are two main problems, both of which might be considered acceptable: 1. A few existing unit-tests for Mochikit.Selector fail, because Sizzle doesn't support some (rarely used) selectors. The list of failed tests is earlier in this thread. 2. The global name space is polluted a bit. If someone is willing to review the patch and either make suggestions how to fix these, or if the ML is ok with both problems - then I can commit it. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Oct 7, 1:49 pm, Arnar Birgisson [EMAIL PROTECTED] wrote: To be clear, I'm basically asking the mailing list: Would you be OK with it if the following selectors would stop working? a[fakeattribute] - i.e. checking for presence of attribute p[lang|=en] - membership test of hyphen-seperated string collections :nth-of-type(...) pseudo-class :enabled, :disabled and :checked pseudo-classes :root pseudo-class My votes... Drop these (not in jQuery, so may be not supported by Sizzle): 1) p[lang|=en] 2) :nth-of-type(...) Rename this: 1) :root to :first Should be there: 1) a[fakeattribute] 2) :enabled 3) :disabled 4) :checked 5) ... more form selectors (see jQuery doc) Regards -- Amit Mendapara --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Tue, Oct 7, 2008 at 19:09, Jason Bunting [EMAIL PROTECTED] wrote: Arnar Birgisson wrote: To be clear, I'm basically asking the mailing list: Would you be OK with it if the following selectors would stop working? a[fakeattribute] - i.e. checking for presence of attribute p[lang|=en] - membership test of hyphen-seperated string collections :nth-of-type(...) pseudo-class :enabled, :disabled and :checked pseudo-classes :root pseudo-class Sounds good to me - would it be too hard to hook into Sizzle and add these ourselves so that the tests pass? I don't know squat about the current code and what is involved in doing this; would it be much work? I intend to find out tonight :) cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Arnar Birgisson wrote: To be clear, I'm basically asking the mailing list: Would you be OK with it if the following selectors would stop working? a[fakeattribute] - i.e. checking for presence of attribute p[lang|=en] - membership test of hyphen-seperated string collections :nth-of-type(...) pseudo-class :enabled, :disabled and :checked pseudo-classes :root pseudo-class Sounds good to me - would it be too hard to hook into Sizzle and add these ourselves so that the tests pass? I don't know squat about the current code and what is involved in doing this; would it be much work? Jason --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hey all, Experimental version ready in seperate branch, see below. On Tue, Oct 7, 2008 at 19:11, Arnar Birgisson [EMAIL PROTECTED] wrote: Sounds good to me - would it be too hard to hook into Sizzle and add these ourselves so that the tests pass? I don't know squat about the current code and what is involved in doing this; would it be much work? I intend to find out tonight :) As it turns out, most of these were easy to implement. Sizzle actually pulls some implementation from jQuery if it is available. I copied and adapted the relevant parts from jQuery (which is also MIT licensed). Sizzle had a few bugs (or at least incorrect behaviour imo) which were easy to fix, among those was support for the |= selector. I created a branch called selector_sizzle [1] and committed this there, ready for review - with the following notes: [1] http://svn.mochikit.com/mochikit/branches/selector_sizzle 1. All unit tests pass in FF except those for three types of selectors. As they were stopping the test suite I had to comment them out. The three selector types that do not work are: - :nth-of-type(...) This one is not supported by Sizzle, and it is not straight forward to do so. I think we should just accept that, they are CSS3 features and probably not used a lot. - a[someattribute] Now this is quite common, and I think that it is the intention of Sizzle to support this. However, the implementation is broken - I couldn't figure out what was wrong so I emailed John about it, I'm currently waiting for a reply. If nothing else, MochiKit should thus lead to one bugfix in Sizzle. :) My opinion here is to wait for John, either he fixes it or helps me to do it. - :root This selector is also from CSS3 and is not supported by Sizzle. I think it is possible, I tried adding support for it but it didn't work out too nicely. This has to do with the fact that Sizzle assumes one is working with document (the main doc) but MochiKit.Selector, and thus the unit test for :root, tries to use this on other documents with the functionality in MochiKit.DOM (withDocument, currentDocument etc.). My opinion is that we should simply skip this as well. 2. The change in the branch introduces a few global names. Namely: chunker, cache, Sizzle, Expr, makeArray, dir, dirNode, dirCheck. This is a bit messy since many of these names are non-distinct and could be used by users or other libraries. We should probably enclose them somehow in the MochiKit.Selector namespace. However, I will suggest to John that Sizzle only binds one global name (Sizzle) and tucks the others away in that namespace. Some unit tests fail on Safari, will have to look at that tomorrow (unless someone else wants to give it a go). Please run the unit tests on other platforms and browsers as I only have access to FF3 and Safari 3. My speed measures, using the SlickSpeed framework, indicate a 55x speedup - going from 3.9 sec with the old Selector module to 71 ms for the new Sizzle based one. The SlickSpeed benchmark tests many selectors, but far from all of them (i.e. it does not has enough coverage to be a unit test). Please run the benchmark on your browser if you like, just visit [2] and everything should be set up already. [2] http://www.hvergi.net/arnar/public/sizzle/speed/# So.. please try this out and comment. What changes do you think are necessary (if any), including my suggestions above, before comitting this to trunk? cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Any update on this? According to slide 92 of John Resig's recent JavaScript Library Overview slides from Ajax Experience '08 (http://www.slideshare.net/jeresig/javascript-library-overview- presentation/), this is being integrated into MochiKit. On Aug 25, 10:06 am, Arnar Birgisson [EMAIL PROTECTED] wrote: Hey all, Some of you may have seen on reddit that John Resig (of jQuery) is working on a new, ultra-fast, css selector module. It is calledSizzle and although it is not released yet, John uploaded a version to github:http://github.com/jeresig/sizzle/tree/master MochiKit's Selector module (which is ported from early versions of Prototype) is unbearably slow, and thus many people steer clear of it. I asked John about the possibility of includingSizzlein MochiKit and he's ok with that,Sizzlewill be released under the MIT license. I did a quick test, just deleted most of the Selector module and replaced with John's code, and modified the exported functions of the Selector module to use that instead. The MochiKit.Selector.Selector object has to go though, so this would not be an entirely backwards-compatible change. The functions findChildElements, findDocElements and $$ would be unchanged though. You can check out the speed test (included withSizzle) where I've added both the trunk version of MochiKit and the MochiKit+Sizzle fusion here:http://www.hvergi.net/arnar/public/sizzle/speed/# For this benchmark, regular MochiKit completed all tests in 3983 milliseconds. The MochiKit+Sizzlecombination does it in 61. That means we are talking about a speedup by a factor of roughly 65! It doesn't come without faults though.Sizzledidn't support all queries in MochiKit's unit tests, namely these are the ones that fail (I'm cc-ing John in case he wants to add support for any of them): a[fakeattribute] - i.e. checking for presence of attribute p[lang|=en] - membership test of hyphen-seperated string collections :nth-of-type(...) pseudo-class :enabled, :disabled and :checked pseudo-classes :root pseudo-class This change would increase the size of the packed version by about 1700 bytes (currently at 173.5 KB). Now, how do people feel about committing a change like this to the trunk? Of course, we'd wait until a fairly stable version ofSizzleis released. John told us thatSizzlewill become the main selector engine behind jQuery, but will also remain a standalone component. All bugfixes will be backported toSizzlealso. As long as MochiKit keeps up, this means we'd benefit from the bugs reported by the jQuery community. A rough test, just plomping in theSizzlesource code into Selector.js is available on my website:http://www.hvergi.net/arnar/public/mochikit/MochiKit/Selector.js cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
On Thu, Aug 28, 2008 at 3:04 AM, Arnar Birgisson [EMAIL PROTECTED] wrote: On Thu, Aug 28, 2008 at 05:35, Amit Mendapara [EMAIL PROTECTED] wrote: Another problem to which I am not sure is licensing issue, Sizzle is MIT licensed while MochiKit has dual MIT/AFL license. I myself thinking of releasing my code under any FSF approved licenses like MIT or GLP. Do you see any difficulties of including these sources to MochiKit particularly under AFL? To be honest, I don't know. Bob, or anyone, do you know if there are problems with this? If there are, John Resig was keen on the idea so he might make some arrangements for us. I don't think there should be a problem with that, since AFL doesn't give any permissions that don't already exist in MIT, but IANAL. -bob --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Sorry, I am bit late to this discussion. It seems promising. As I said in another post, I am implementing jQuery style Query module which is currently using MochiKit.Selector (the code is not made public yet). As my proposed module is totally new, I'm not afraid of backward compatibility. That's why I have decided to implement new selector for the Query module. Though if MochiKit.Selector is going to be Sizzle based, it would be great... Another problem to which I am not sure is licensing issue, Sizzle is MIT licensed while MochiKit has dual MIT/AFL license. I myself thinking of releasing my code under any FSF approved licenses like MIT or GLP. Do you see any difficulties of including these sources to MochiKit particularly under AFL? Regards .. Amit Mendapara On Aug 25, 9:06 pm, Arnar Birgisson [EMAIL PROTECTED] wrote: Hey all, Some of you may have seen on reddit that John Resig (of jQuery) is working on a new, ultra-fast, css selector module. It is called Sizzle and although it is not released yet, John uploaded a version to github:http://github.com/jeresig/sizzle/tree/master MochiKit's Selector module (which is ported from early versions of Prototype) is unbearably slow, and thus many people steer clear of it. I asked John about the possibility of including Sizzle in MochiKit and he's ok with that, Sizzle will be released under the MIT license. I did a quick test, just deleted most of the Selector module and replaced with John's code, and modified the exported functions of the Selector module to use that instead. The MochiKit.Selector.Selector object has to go though, so this would not be an entirely backwards-compatible change. The functions findChildElements, findDocElements and $$ would be unchanged though. You can check out the speed test (included with Sizzle) where I've added both the trunk version of MochiKit and the MochiKit+Sizzle fusion here:http://www.hvergi.net/arnar/public/sizzle/speed/# For this benchmark, regular MochiKit completed all tests in 3983 milliseconds. The MochiKit+Sizzle combination does it in 61. That means we are talking about a speedup by a factor of roughly 65! It doesn't come without faults though. Sizzle didn't support all queries in MochiKit's unit tests, namely these are the ones that fail (I'm cc-ing John in case he wants to add support for any of them): a[fakeattribute] - i.e. checking for presence of attribute p[lang|=en] - membership test of hyphen-seperated string collections :nth-of-type(...) pseudo-class :enabled, :disabled and :checked pseudo-classes :root pseudo-class This change would increase the size of the packed version by about 1700 bytes (currently at 173.5 KB). Now, how do people feel about committing a change like this to the trunk? Of course, we'd wait until a fairly stable version of Sizzle is released. John told us that Sizzle will become the main selector engine behind jQuery, but will also remain a standalone component. All bugfixes will be backported to Sizzle also. As long as MochiKit keeps up, this means we'd benefit from the bugs reported by the jQuery community. A rough test, just plomping in the Sizzle source code into Selector.js is available on my website:http://www.hvergi.net/arnar/public/mochikit/MochiKit/Selector.js cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi again, On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer [EMAIL PROTECTED] wrote: ... I noticed that your addition fixes the errors in MochiKit.Selector. I just realized, do you mean you noticed that I fixed the errors that were causing the tests in Selector to fail? If so, then no.. i fixed them by commenting out the tests that were failing. They were stopping the test-suite from continuing to other tests. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Selector speedup by using John Resig's Sizzle
Hi again, On Mon, Aug 25, 2008 at 16:06, Arnar Birgisson [EMAIL PROTECTED] wrote: For this benchmark, regular MochiKit completed all tests in 3983 milliseconds. The MochiKit+Sizzle combination does it in 61. That means we are talking about a speedup by a factor of roughly 65! Sorry, forgot to mention that this is running on my computer on Firefox 3.0. For browsers that support the querySelectorAll method (such as FF 3.1), I'd expect an even greater speedup. cheers, Arnar --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups MochiKit group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
