Discussion:
Selector speedup by using John Resig's Sizzle
Arnar Birgisson
2008-08-25 16:06:18 UTC
Permalink
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-08-25 16:11:18 UTC
Permalink
Hi again,
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Chris Lee-Messer
2008-08-26 13:38:55 UTC
Permalink
Adding Sizzle sounds like a substantial plus, I noticed that your
addition fixes the errors in MochiKit.Selector. Does it break any of
the other (non-selector related) tests in MochiKit?

-Chris

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.
Post by Arnar Birgisson
Hi again,
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-08-26 14:16:10 UTC
Permalink
Hello Chris,

On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer
Post by Chris Lee-Messer
Adding Sizzle sounds like a substantial plus, I noticed that your
addition fixes the errors in MochiKit.Selector. Does it break any of
the other (non-selector related) tests in MochiKit?
No, no tests outside the Selector module are broken. You can run the
test suite here:
http://www.hvergi.net/arnar/public/mochikit/tests/

There is some namespace pollution, i.e. the Sizzle code defines a few
globals. Maybe we should hide them inside some
MochiKit.Selector.__internal__ object or similar?

The Selector module has limited usability now imo, so I think this
would be a good move even though it breaks backwards compatibility a
bit.
Post by 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.
Yes, I noticed the same when I ran the benchmark, but not so drastically.

I added a second instance Sizzle to the test suite, at the very end.
That one gives lower times so indeed there seems to be some caching of
compiled code going on.

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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-08-26 16:14:52 UTC
Permalink
Hi again,

On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer
... 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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-20 10:07:02 UTC
Permalink
Hi all,
Post by Arnar Birgisson
On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer
Post by 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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-20 14:47:11 UTC
Permalink
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
Post by Arnar Birgisson
Hi John,
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.
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-20 15:01:54 UTC
Permalink
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
Post by Per Cederberg
http://ejohn.org/blog/queryselectorall-in-firefox-31/
And in fact, there is a slight bug in Sizzle here, causing it to not
Sizzle("...", document)
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
Post by Arnar Birgisson
Hi John,
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.
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-20 15:31:31 UTC
Permalink
Hi again,
Post by John Resig
Wow, thanks for catching that. That's amusing that it was still so
fast without using querySelectorAll, in that case. Just committed the
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-20 15:44:07 UTC
Permalink
Post by Arnar Birgisson
Well, that makes a big difference :) Now both MK+Sizzle and Sizzle
standalone do the benchmark in 53 ms.
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 :)
Ha! I certainly won't complain with those types of numbers.

It's funny, I was struggling to find out why some of my selectors were
slower when qSA was supposed to be used - I didn't realize that it was
a product of how the test suite was working (it always passes in the
extra document argument) and how the perf suite works (it doesn't pass
in document). Oh well, all better now!

--John

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-20 14:43:17 UTC
Permalink
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
Post by Arnar Birgisson
Hi John,
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.
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-20 15:19:01 UTC
Permalink
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
Post by Arnar Birgisson
Hi again,
Post by John Resig
Just to clarify: Are you turning on JIT in 3.1?
JIT is off.
Post by John Resig
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-20 15:25:44 UTC
Permalink
Hi John,
Post by John Resig
http://github.com/jeresig/sizzle/commit/93e33dc2a41e2b0aa0e1e1c66368f5d224da80e1
Excellent. Thanks!
Post by John Resig
The exception is :visible and :hidden - which should be handled by the
host library.
I agree.
Post by John Resig
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-20 15:40:42 UTC
Permalink
Post by Arnar Birgisson
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.
Post by Arnar Birgisson
[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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-20 16:13:07 UTC
Permalink
Post by John Resig
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-23 15:37:47 UTC
Permalink
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
Post by Arnar Birgisson
Post by John Resig
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-23 16:12:40 UTC
Permalink
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
Post by John Resig
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.
- 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
Post by Arnar Birgisson
Post by John Resig
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-23 18:32:26 UTC
Permalink
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
Post by John Resig
http://ejohn.org/apps/sizzle/test/
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
Post by John Resig
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.
- 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
Post by Arnar Birgisson
Post by John Resig
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-23 19:28:40 UTC
Permalink
Post by Per Cederberg
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 :-/
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Giulio Cesare Solaroli
2008-11-03 10:56:50 UTC
Permalink
Hello,

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.

Regards,

Giulio Cesare
Post by John Resig
Post by Per Cederberg
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 :-/
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-11-04 10:36:48 UTC
Permalink
Hello Giulio Cesare,

First: wow - your name is awesome :)

On Mon, Nov 3, 2008 at 11:56, Giulio Cesare Solaroli
Post by Giulio Cesare Solaroli
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-11-18 22:04:28 UTC
Permalink
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
Post by Arnar Birgisson
Hello Giulio Cesare,
First: wow - your name is awesome :)
On Mon, Nov 3, 2008 at 11:56, Giulio Cesare Solaroli
Post by Giulio Cesare Solaroli
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-11-20 10:55:49 UTC
Permalink
Hi Per,
Post by Per Cederberg
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.
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-11-24 08:33:24 UTC
Permalink
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
Post by Arnar Birgisson
Hi Per,
Post by Per Cederberg
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.
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

Arnar Birgisson
2008-10-20 14:05:28 UTC
Permalink
Hi John,
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
John Resig
2008-10-20 13:52:38 UTC
Permalink
That's... odd. Are there any selectors that are noticeably faster?
Maybe something is failing?

--John
Post by Arnar Birgisson
Hi all,
Post by Arnar Birgisson
On Tue, Aug 26, 2008 at 13:38, Chris Lee-Messer
Post by 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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
JasonBunting
2008-08-25 17:02:25 UTC
Permalink
Sounds good to me!
Post by Arnar Birgisson
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
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Bob Ippolito
2008-08-28 16:31:59 UTC
Permalink
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-08-28 05:35:31 UTC
Permalink
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
Post by Arnar Birgisson
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
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-08-28 10:04:08 UTC
Permalink
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.

cheers,
Arnar

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
JasonBunting
2008-10-07 04:21:20 UTC
Permalink
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.
Post by Arnar Birgisson
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
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-07 07:24:33 UTC
Permalink
Hi all,
Post by JasonBunting
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-10-07 08:34:05 UTC
Permalink
Hello Arnar,

If you have noticed, I have published MochiKit.Query module on
Launchpad (https://launchpad.net/mochikit-ext). The goal of this
module is to provide functionalities similar to jQuery (not 100%
compatible). Currently, it uses MochiKit.Selector. Initially, I was
thinking on implementing jQuery style selector but then I come across
to this post and I changed my mind to wait for the integration of
Sizzle in MochiKit.

It seems that you are trying to make the integration into existing
MochiKit.Selector module with full backward compatibility. I think,
you should integrate Sizzle as a separate MochiKit.Sizzle module. And
we should kindly request Mr. Bob to deprecate MochiKit.Selector (or
just remove it as 1.4 is not officially released).

Regards
--
Amit Mendapara
Post by Arnar Birgisson
Hi all,
Post by JasonBunting
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,
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-07 08:49:51 UTC
Permalink
Hi all,
Post by Amit Mendapara
It seems that you are trying to make the integration into existing
MochiKit.Selector module with full backward compatibility. I think,
you should integrate Sizzle as a separate MochiKit.Sizzle module. And
we should kindly request Mr. Bob to deprecate MochiKit.Selector (or
just remove it as 1.4 is not officially released).
Personally, I don't mind if the change is backwards incompatible - but
it's not my decision alone if it is ok. I would greatly prefer to keep
Mochikit.Selector - because someday something better than Sizzle might
come along.

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

cheers,
Arnar

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-10-07 11:02:59 UTC
Permalink
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-07 11:23:37 UTC
Permalink
Post by Amit Mendapara
1) a[fakeattribute]
2) :enabled
3) :disabled
4) :checked
5) ... more form selectors (see jQuery doc)
Right, last time I looked at this was with a pre-release of Sizzle, so
things might have improved. I will update to the latest Sizzle version
when I get home tonight and prepare a patch.

cheers,
Arnar

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-07 17:11:41 UTC
Permalink
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-07 23:08:34 UTC
Permalink
Hey all,

Experimental version ready in seperate branch, see below.
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-08 13:49:04 UTC
Permalink
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.

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!"

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)...

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?

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.

6. There seems to be an unnecessary whitespace change in 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)

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;

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.

10. I'm not afraid of regexps, but the "chunker" one in the code is
just ridiculous. It is much too dense to be in any kind of production
code without proper comments and/or explanations as to its function
and use.

11. This line (118):

if ( parts.length > 0 && !checkSet ) {

should be just:

if ( parts.length > 0) {

it seems.

... and now I've got to take a break. This email is already quite
long. I'll continue looking at the code some other day.

Cheers,

/Per

PS. I just discovered that Google Groups silently dropped all my
emails that used another sender address, so I'm currently resending
all my recent postings. Hence the sudden email bombing...

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-08 14:12:25 UTC
Permalink
Hi Per,
Post by Per Cederberg
This is great work! But since I just took on my code-review-glasses,
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.
Post by Per Cederberg
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.
Post by Per Cederberg
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.
Post by Per Cederberg
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.
Post by Per Cederberg
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.
Post by Per Cederberg
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?
Post by Per Cederberg
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.
Post by Per Cederberg
8. The Sizzle result cache seems to break if people start manipulating
return cache && doCache ?
results;
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.
Post by Per Cederberg
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 features not supported
by it (and take the feature penalty).
Post by Per Cederberg
10. I'm not afraid of regexps, but the "chunker" one in the code is
just ridiculous. It is much too dense to be in any kind of production
code without proper comments and/or explanations as to its function
and use.
This is another comment for John I guess. I don't necessarily agree
with you though, its function was relatively clear after pasting it
into http://regexpal.com/ and playing with it for a minute.
Post by Per Cederberg
if ( parts.length > 0 && !checkSet ) {
if ( parts.length > 0) {
it seems.
Right, there are several "bugs" of this kind in Sizzle already.
Post by Per Cederberg
... and now I've got to take a break. This email is already quite
long. I'll continue looking at the code some other day.
Thank you very much for excellent, detailed comments. Reviewing is
just as hard as coding, but more boring :)

cheers,
Arnar

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
j***@gmail.com
2008-10-08 14:41:36 UTC
Permalink
Hey Everyone -
Post by Arnar Birgisson
Ah, ok. I just thought the test was buggy. != is not actually defined
"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]).
Post by Arnar Birgisson
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?
Post by Arnar Birgisson
I would also vote against pushing this to a release as it stands now.
Naturally - IE support is still a minefield.
Post by Arnar Birgisson
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.
Post by Arnar Birgisson
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.
Post by Arnar Birgisson
Post by Per Cederberg
8. The Sizzle result cache seems to break if people start manipulating
  return cache && doCache ?
      results;
  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.
Post by Arnar Birgisson
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.
Post by Arnar Birgisson
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.
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-08 14:53:31 UTC
Permalink
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?
Post by j***@gmail.com
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.
Post by j***@gmail.com
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.
Post by j***@gmail.com
Post by Arnar Birgisson
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.
Post by j***@gmail.com
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-08 16:07:26 UTC
Permalink
Post by Arnar Birgisson
Ah, ok. I just thought the test was buggy. != is not actually defined
"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.
I'm used to XPath semantics, which seem to require existence. Now, if
everybody else use a different interpretation, that is of course ok
with me.
Post by Arnar Birgisson
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 run IE 6 in CodeWeavers CrossOver Mac. Quite nice to have the full
battery of browsers available... (except IE 7 though)
Post by Arnar Birgisson
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.
Naturally, but I would have to fix the old implementation for the 1.4
release. Or not include it at all. Or document it as "experimental".
Or something.
Post by Arnar Birgisson
Post by Per Cederberg
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?
My bad. I got confused by the web diff view in Trac.
Post by Arnar Birgisson
That is probably a mistake, it should definitely be in the docs. I
will fix this in the trunk.
Great, thanks!
Post by Arnar Birgisson
Post by Per Cederberg
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 features not supported
by it (and take the feature penalty).
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.

We really need to make sure that our test suite has good test
coverage. Then if we encounter issues in Safari, we can handle those
by searching for those patterns in the query before calling
querySelectorAll.
Post by Arnar Birgisson
Post by Per Cederberg
10. I'm not afraid of regexps, but the "chunker" one in the code is
just ridiculous. It is much too dense to be in any kind of production
code without proper comments and/or explanations as to its function
and use.
This is another comment for John I guess. I don't necessarily agree
with you though, its function was relatively clear after pasting it
into http://regexpal.com/ and playing with it for a minute.
Possibly. But I'd still like some comments... :-)

BTW, John. I saw your recent Sizzle commit and commented it on github.

Cheers,

/Per

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-08 19:28:48 UTC
Permalink
A little status report.
Post by Per Cederberg
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
<div><div><p>...</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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-10-13 12:43:02 UTC
Permalink
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-13 12:48:08 UTC
Permalink
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-13 12:52:23 UTC
Permalink
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
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-13 13:00:20 UTC
Permalink
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-10-13 13:13:15 UTC
Permalink
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-13 13:32:49 UTC
Permalink
On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-13 13:24:58 UTC
Permalink
On Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-10-13 13:47:26 UTC
Permalink
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 Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Bob Ippolito
2008-10-13 15:19:20 UTC
Permalink
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
Post by Amit Mendapara
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 Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-13 15:30:33 UTC
Permalink
Post by Bob Ippolito
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Amit Mendapara
2008-10-14 07:51:38 UTC
Permalink
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
Post by Bob Ippolito
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
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 Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Bob Ippolito
2008-10-14 08:47:11 UTC
Permalink
Well clearly we have a place where all of that can happen - the mailing list :P

On Tue, Oct 14, 2008 at 12:51 AM, Amit Mendapara
Post by Amit Mendapara
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
Post by Bob Ippolito
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
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 Mon, Oct 13, 2008 at 3:13 PM, Amit Mendapara
Post by Amit Mendapara
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-15 08:15:55 UTC
Permalink
I've now updated the docs for MochiKit.Selector to include some
Post by Arnar Birgisson
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
<div><div><p>...</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.
Post by Arnar Birgisson
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

The remaining issues seem to be feature omissions in the current
MochiKit.Selector implementation, so I'll just ignore those.

Cheers,

/Per

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-15 08:21:41 UTC
Permalink
Hi Per,
Post by Per Cederberg
Post by Arnar Birgisson
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
<div><div><p>...</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.
Post by Per Cederberg
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-15 11:01:41 UTC
Permalink
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-15 11:37:05 UTC
Permalink
Post by Arnar Birgisson
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. :-(
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Arnar Birgisson
2008-10-15 11:52:29 UTC
Permalink
Post by Per Cederberg
Post by Arnar Birgisson
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.
Post by Per Cederberg
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
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.
Post by Per Cederberg
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Per Cederberg
2008-10-15 10:46:42 UTC
Permalink
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
Post by Arnar Birgisson
Hi Per,
Post by Per Cederberg
Post by Arnar Birgisson
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
<div><div><p>...</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.
Post by Per Cederberg
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Jason Bunting
2008-10-07 17:09:20 UTC
Permalink
Post by Arnar Birgisson
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 ***@googlegroups.com
To unsubscribe from this group, send email to mochikit+***@googlegroups.com
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
Loading...