mootools / slick

Standalone CSS Selector Parser and Engine. An official MooTools project.
MIT License
149 stars 22 forks source link

Recursive matcher that doesnt do .search over whole document to match com #40

Closed Inviz closed 13 years ago

Inviz commented 13 years ago

Here you guys are. (from https://github.com/mootools/slick/issues/39) #39

on https://github.com/lovelyscalabledrawings/slick/ master branch you can find a benchmark.

The gist of it is:

https://gist.github.com/933639

Note: My matchNode implementation uses old approach of nativeMatchSelector first and then does the recursive thing. nativeMatchSelector part may be easily removed (as shown in benchmarks)

@markiz@cpojer @arian ping

Inviz commented 13 years ago
   // Old master branch slick (uses nativeMatchSelector)
   Oldest Slick x 5,315 ±6.98% (81 cycles)

   // current develop (doesnt use nativeMatchSelector?)
   Old Slick x 218 ±13.00% (63 cycles)

   // patched
   Current Slick x 4,738 ±12.04% (77 cycles)
   Fastest is Oldest Slick
   failed tests 0

   // current develop - (yes, it doesnt use nativeMatchSelector)
   Old Slick (no nativeMatchesSellector) x 161 ±11.70% (75 cycles)
   // patched
   Current Slick (no nativeMatchesSellector) x 4,423 ±9.52% (94 cycles)
   Fastest is Current Slick (no nativeMatchesSellector)
   failed tests 0
fabiomcosta commented 13 years ago

Inviz try the last commit i made, just type make and it will run the specs on your default browser. I noticed 14 specs failing on chrome last stable version even without your patch, so take them into consideration (they might be wrong specs).

Please run the specs and tell us the result. I tried pulling your changes and it didnt work, gave some conflicts, maybe your branch should be rebased with the official one.

Thanks

Inviz commented 13 years ago

Come on it's just one file changed. Conflcits? What's the point of pull request if i do the merge myself?:)

Inviz commented 13 years ago

So i didnt have the latest develop after all. Checked on upstream:

Only these two fail:

 expect( context.MATCH(testNode, "[attr"+ operator +"'"+ value +"']") ).toEqual(shouldBeTrue ? true : false);

and

     expect( context.MATCH(testNode, document.createElement('div')) ).toEqual(false);

Not sure what's the deal in those two failed, can you help me figure out?

I think there could be more deep matching tests because it's not embarassing anymore.

Inviz commented 13 years ago

I reworked the pull, should not have any conflicts now:

https://github.com/mootools/slick/pull/41