New registry

[rvagg]

Following on from #19 (comment)

Critique away.

I'll try and come up with some benchmarks when I have a bit more free time, I'm happy to take suggestions on exactly what you'd like to see benchmarked tho.

Cheers.

[fat]

I think simple benchmarks for all the public api methods would be a good sanity check. Make sure that attaching/removing events doesn't take like 100x as long if you've attached 1000 events. Things like that.

[rvagg]

also, it's fairly comment-free at the moment, I'm intending to put notes in about some of the more non-obvious things to aid maintainability.

[fat]

sounds good

[fat]

created a benchmark test thing tonight here 6bd6861

I'm going to run it through a few of the major browsers tomorrow - some interesting things already popping up!

I'll try to run your branch through tomorrow as well and do some comparisons.

[rvagg]

Well seeing that you've progressed in this area I thought I'd better check-in and push all my local changes because I've also progressed and we're probably overlapping a bit.

My benchmarks.html takes a different approach to yours, a bit simpler but able to compare current-bean to my new-bean and also throw in NWEvents and jQuery as well to see what it's up against (even though they all take different approaches so it's not quite apples-to-apples, but it does point out the weak points).

My original version didn't perform particularly well so I went searching for optimisations and found plenty, both in my code and across the rest of Bean. I've also done some other minor clean-ups which largely boil down to questions of style, but I couldn't help it.

Benchmarks across browsers are a hugely mixed-bag, although I guess it should be no surprise since we're dealing so much with natives. Custom events on DOM elements is greatly improved on all browsers but that's largely to do with commit b644594 and some other work on fixEvent() which turns out to be an absolute pig.

Regarding the rest, the biggest wins are visible in Chrome and Safari where my latest code runs roughly 20% faster on average in the different tests. The biggest losers are Internet Explorer 9 and Opera but the results are harder to summarise on those. The one that stands out is the plain click-on-DOM test in IE9 where my code runs a bit more than 1/2 the speed of current Bean, but my code still runs faster than NW & jQuery--which is all very strange when you consider what that test does in all other browsers, I can't help but think that something is amiss with IE9.
Looking at IE6,7,8 is a different story, of course. On average I seem to have got a speed boost but there are some areas where current Bean is a bit faster, but not by huge margins. I think this is good news.
Firefox is a bit in the middle although on average my code appears to be slightly slower. Older Firefox is a bit different but mostly irrelevant now so not worth analysing.

I was planning on adding one more test to the benchmark suite, one where multiple event types are used, multiple custom and multiple native, to better reflect normal usage. Because I'm storing entries in the registry by event name/type I was suspecting I'd see a slight boost in this situation because the other tests use a single event name.

But now I have this all checked in I might check yours out and have a look.

[rvagg]

this was in the noConflict PR I put in, I needed noConflict() to work so I could test bean.js (old) against src/bean.js (new).

[rvagg]

After having put that comment in there about the benchmark numbers I realised I went and put back in some more realistic numbers that I had before trying to make old IE happy, something like:

var maxms = 500, listeners = 100, elements = 20, fires = 20

So there's 100 listeners per iteration and more iterations are run. listeners could even be increased to see how lookups are performing. Generally (and surprisingly) you don't see too much change in the scale of the speed differences across modern browsers but the IE9 picture starts to clear up a bit and you don't get the wild differences that I commented on previously.

[fat]

Awesome glad to see you were thinking this through! I think i'd rather stick to using benchmark.js - @jdalton has been working alot on this stuff, so I imagine there's some trickiness in the lib worth leveraging. Shouldn't be too hard to add other libs to the benchmark.html page I've already setup.

[rvagg]

I've updated this branch as bean-rvagg-wip in NPM, it'd be great if others want to try this out to confirm that all's well in real-world use cases.

[jdalton]

@fat lemmie know if ya have any usage tips/tweaks or issues w/ benchmark.js

[fat]

@jdalton I will definitely - so far really great stuff :)

[rvagg]

Updated bean-rvagg-wip in NPM, I think the last version didn't have a proper build so it was running old-bean. Working well for me at the moment.
My latest commits remove the layerX/layerY deprecation debug messages you get in Chrome when using Bean by the way.

[rvagg]

Work-in-progress but I've updated benchmark.js-based benchmarks to be multi-library and compare old/new.
Interesting, it's a bit too atomic for my liking, I'd like to add at least one general benchmark that does some combination of add/fire/remove together to see the scale of the aggregate performance differences (I still prefer my more naive benchmarks!).

I'm also having a ton of trouble getting anything meaningful out of older browsers and even IE9 is having difficulty running these; @jdalton if you're looking for something to geek out on then you could have a poke around in there for us. Certainly one thing that you might want to look at is I'm getting errors in FF3.6 (haven't tried anything between that and Aurora just yet), it's complaining about element not being defined within the first test executor. It's my understanding that setup / execute / teardown are stringified, appended together with timing functions and executed. Since we have var element = .... in the setup, why might FF3.6 have a problem with it? (this is a problem for fat/master as well as rvagg/newregistry).

[jdalton]

@rvagg FF 3.6 is erroring on yur setup/teardown compiling because it doesn't support Function#toString correctly so we fallback to executing setup() and tear() w/o compiling. If you want to compile in FF3.6 you can use string values as the setup, teardown, fn properties.

For older IE's you may need to apply a patch to avoid long script warnings:
http://jsperf.com/faq#script-warnings

For IE also add the nano applet to your test page. That will allow the tests to run shorter.

Also you should avoid iterating inside a test. These tests will be repeated a lot already without the need for for-loops in yur tests.

[rvagg]

Thanks for all the good info.
I added iteration because I wanted to test longer lists of handlers, I was assuming that it'd do a setup(),execute(),teardown() which wouldn't test what I wanted, this isn't the case? Will it do more than one execute() between the setup() and teardown()?

[jdalton]

@rvagg The test setup/teardown is explained in the documentation with examples of how things compile. (I'll be sure to add a note about how we handle compiling and the strengths and weaknesses of different ways to compile)
That should clear some things up for you.

Also be careful of tests which append or remove elements to the document because this could happen hundreds of thousands of times in a test. In yur setup and teardown properties you can reference this.count to have access to the current iteration count for the test. This will allow you to know exactly how many elements to add before a test which removes elements or how many elements to remove after a test which adds elements.

[rvagg]

OK, I'm pretty happy with where this is for now. IMO speed is decent enough considering the change, but that's a matter of opinion. remove() could do with some work to remove the double-dip of the registry but that's not so straightforward, a task for another time.

I haven't had time to run the benchmarks as they are now on older browsers but there are 3 more at the end now that do an add/fire/remove sequence. The remove() benchmarks were commented out after I couldn't come up with a sensible way to run them (as per the discussion above and what's been said on #44).

A couple of additional thoughts I've had on performance:

• We could take a hybrid approach to attaching an ID by doing it just on DOM elements but not on other objects. IMO while it's not as clean as a pure internal approach, it's a much lesser evil augmenting DOM nodes than other people's objects; but again perhaps a matter of opinion. Unfortunately this wouldn't be a trivial job to change from the registry I've implemented in there and it would add a bit more weight to the code.
• We could take a similar approach to jQuery to speed up handlers of the same type on the same object--by only adding a single handler to the underlying object for any particular type and then delegating to all additional handlers for that object/type from there. As you can see from the benchmarks for 'add' vs 'add unique' it can make quite a difference. Unfortunately code bloat awaits those who fear to tread this path.

Feel free to comment in the diff and I'll try and address any concerns.

Oh, and if you're going to accept this PR, since it's so major, we could switch coding style to semi-colon-free and comma-first if you wanted; it looks like that's your current preferred style.

[fat]

I'm going to try to run through this next week... i'm currently swamped at work, but I am very much looking forward to digging into this. Keeping this project really lean is important to me as it does a great job for people developing 3rd party js.

[ded]

i'd love to finally get this in @fat. plus the non-standard layerXY is getting super annoying to look at in the chrome console

[rvagg]

But the delay is understandable. If I had an OS project with 12,549 watchers and 2,161 forks, I think I know where I'd be spending my free time!

[rvagg]

Since I've been reminded of this I decided to go ahead and finish up the other things I wanted to do with this branch.

I've changed the coding style to comma-first & sans-semicolons which I'm guessing you'll be cool with. I've put in a few extra JSHint options and reorganised the code slightly so that things are defined before they are used in the source, I've reformatted and renamed a few things just for readability sake without sacrificing minified size. I'm certainly finding the current version really easy to look at.

The diff isn't very helpful to look at now unfortunately. If I've gone too far here let me know, I didn't want to make it harder to review the code but I thought I may as well go the whole hog as long as I'm making major changes.

I also merged @paulredmond's changes in #47 which basically makes fixEvent() copy properties just like the latest jQuery (i.e. copy only what's required and nothing more--makes it speedy). My previous version had a whitelist but applied that list to all event types, this new method has a special whitelist for the different event types.

Current Bean is 2558 bytes, minified and gzipped, my latest with registry is 3089.

I've published a new bean-rvagg-wip to NPM with these changes if anyone wants to have a go. I'm currently running it against a Bean (and Bootstrap) dependent product in-house with no issues.

[ded]

@rvagg that's good news. i'd love to use your temporary published package

[paulredmond]

Awesome! Great job @rvagg

[fat]

@rvagg i'm thinking more I should just give you commit access on this project - is that cool? As you pointed out, i'm definitely swamped - and it's not that this isn't a super important project.

We're currently using bean for tweet embeds (which are everywhere now), plus it's used by disqus, storify, and nearly every other major embedded project.

If your interested let me know, and then you can merge as you see fit - how does that sound?

[ded]

hooray

[rvagg]

Sure, commit access would be great!

[fat]

alright - you should have commit access - that doesn't mean go crazy though! Remember, if you break bean, you break the internet. :)

[rvagg]

awesome, I'll have to think up some clever subtle ways to break the internet then that don't make it look like my fault.

[rvagg]

FYI I'm not going to merge this straight away, I think we need some tests for custom events (mouseenter/mouseleave). It looks like they might not be working quite right with the new code, in 2 areas: fixEvent() not copying all the right properties to the final event object and the remove() not removing properly.

So, anyone who wants to be using the new stuff (to test it out in the real world!) should still try bean-rvagg-wip from NPM, or grab the source from https://github.com/rvagg/bean/blob/newregistry/src/bean.js if you're not using Ender.

[rvagg]

a bunch of fixes for remove() and fixEvent() and a boat-load of additional tests. I added the ability to do bean.remove(element, handler) while I was fixing up remove(), a minor addition.
I also took the liberty of adding event.stop(), something I badly miss when not using Prototype, an alias for both preventDefault() and stopPropagation() at the same time. It's a pretty small addition too thankfully.