Re: Event.fix optimisation

> @Diego
>
> Forgot to tell you, I closed you merge() ticket last night :)
>
> --
> Ariel Fleslerhttp://flesler.blogspot.com
>
> On 13 mayo, 11:39, Diego Perini <diego.per...@...> wrote:
>
> > Brandon,
> > I completely agree with you about the choice of not breaking
> > whatever was present and usable previously. A wise decision.
>
> > Moving the static declaration of the properties and the .split()
> > out of the .fix() method will improve things on all browsers
> > without any risk at all. Really it is safe...
>
> > I look forward to implement next part of the patch for 1.3.
>
> > Keep up the fantastic job you are doing and forget my rants.
>
> > --
> > Diego Perini
>
> > On 13 Mag, 15:35, "Brandon Aaron" <brandon.aa...@...> wrote:
>
> > > Your forgetting something very important... this is a point release. We
> > > don't want to break people's code. Properties like srcElement, toElement,
> > > fromElement, originalTarget are still used by some devs for whatever reason
> > > they may have.
>
> > > I, and everyone else, greatly appreciate your (and everyone else involved)
> > > immense amount of work on bringing light to the optimization needs in the
> > > event system. The event system is a complex beast that has grown to support
> > > a number of scenarios. I needed to hold the balance between the different
> > > ideas/patches to make sure we got things running faster but with the exact
> > > same requirements. I fudged on that a little ... since I'm not doing an
> > > exact prop for prop duplicate of the original event anymore .... however, as
> > > we all know now the slow down was too much of a hit.
>
> > > As to your last question ... I thought I made myself very clear about
> > > several parts of your patch. The reason you have a speed up on native events
> > > is because you aren't normalizing the event object as it should be. You are
> > > doing browser detection which is bad in this case. We should be doing
> > > feature detection _on_ the event object itself as we are currently doing.
> > > Breaking out by browser will just create more code and a maintenance
> > > nightmare in the long run.
>
> > > --
> > > Brandon Aaron
>
> > > On Tue, May 13, 2008 at 6:36 AM, Diego Perini <diego.per...@...>
> > > wrote:
>
> > > > Brandon,
> > > > definitely the new changes you made to events and
> > > > some of the patches that Ariel wrote have improved
> > > > things considerably especially for events in 1.2.4.
>
> > > > If you compare what we had in 1.2.3 with the "extend"
> > > > kludge you will realize that the changes I suggested
> > > > were highly required and necessary for UI / Widgets.
>
> > > > Hope you are not saying I am slowing down events !!!
>
> > > > I still believe there are some extra improvement that
> > > > are easy to do, the first one being to move out of the
> > > > method the static declaration of the properties so they
> > > > don't get re-declared (and re-split) for each new event.
>
> > > > The other is a suggestion to try to shorten-up that list
> > > > leaving just the properties that are strictly necessary,
> > > > it sound to me as a non sense to try to keep a nearly
> > > > common list of properties between IE and the others,
> > > > they should probably be two static lists, one for each
> > > > browser, so each browser just get copied only the
> > > > properties it needs, not the ones from its nearest
> > > > competitor... :-)
>
> > > >    srcElement, toElement, fromElement
>
> > > > I see it dangerous to have new properties called like
> > > > those while working with jQuery and some other mixed
> > > > code environment.
>
> > > > Don't you believe you are going to fold the detection
> > > > methods embedded in other scripts running in
> > > > parallel with jQuery ?
>
> > > > The last addition of the "originalTarget" is something we
> > > > should avoid since it does not give any advantage over
> > > > "target", but as said above I may be wrong and there
> > > > may be an obvious explanation that I missed.
>
> > > > The big improvements for "native" events that where
> > > > present in my patch have been left out, is there
> > > > something wrong with that one part ?
>
> > > > Diego Perini
>
> > > > On May 4, 4:28 pm, "Brandon Aaron" <brandon.aa...@...> wrote:
> > > > > Your test page (
> > > >http://javascript.nwbox.com/JQ_TWEAKS2/draggables/test-draggables-lat...)
> > > > > is using r5225 to test against your patched version. I copied your test
> > > > > exactly but instead used r5358 and consistently got a time that was less
> > > > > than your patched version. I've attached two pngs of the results.
>
> > > > > --
> > > > > Brandon Aaron
>
> > > > > On Sun, May 4, 2008 at 5:57 AM, Diego Perini <diego.per...@...>
> > > > wrote:
>
> > > > > > Brandon,
> > > > > > I was using r5374 which was latest, now I tested r5390 too with the
> > > > > > same tests and the same results and custom/namespaced events
> > > > > > are still 2x faster in Firefox. I tested "draggables" and "slider" and
> > > > > > my previous tests on normal events.
>
> > > > > > Please have a look at the two png of the profiler linked in my post
> > > > > > above and help me understand where I am wrong interpreting it.
>
> > > > > > You didn't specify in which cases you tested latest SVN being faster
> > > > > > than my patched version. Can you send the test you are using so
> > > > > > I can compare and fix ?
>
> > > > > > Diego Perini
>
> > > > > > On 4 Mag, 03:10, "Brandon Aaron" <brandon.aa...@...> wrote:
> > > > > > > Please use the latest SVN version of jQuery to test against. The
> > > > > > > jquery-latest is out of date. I'd be curious to know what your
> > > > results
> > > > > > are
> > > > > > > after doing so. My testing showed that latest svn is faster than the
> > > > > > patched
> > > > > > > version.
>
> > > > > > > --
> > > > > > > Brandon Aaron
>
> > > > > > > On Sat, May 3, 2008 at 7:51 PM, Diego Perini <diego.per...@...
>
> > > > > > wrote:
>
> > > > > > > > weepy,
> > > > > > > > something wrong here. You are correct % time is now dispatched to
> > > > > > > > "create()". That properties copying process existed before
> > > > embedded in
> > > > > > > > the "fix()" method and even before when "extend()" was used, and I
> > > > > > > > believe Brandon says it will survive.
>
> > > > > > > > Don't just look at percentages, those will always sum up to
> > > > 100%...
> > > > > > > > Obviously percentage say us something important, but that's not
> > > > all.
> > > > > > > > Look also at the first line of the profiler snapshot, you will see
> > > > the
> > > > > > > > total number of calls and the time jquery, events and their
> > > > handlers
> > > > > > > > have consumed. From the number of call to "drag()" you can see
> > > > which
> > > > > > > > test was run for a slightly longer time so you can adjust if you
> > > > want.
>
> > > > > > > > I went to the UI.draggables cut & paste the sampe I found there
> > > > and
> > > > > > > > here are my tests using "jquery-latest.js" and my patched-r5374
> > > > > > > > version:
>
> > > > > > > >  http://javascript.nwbox.com/JQ_TWEAKS2/draggables/index.html
>
> > > > > > > > with relative screen shots of the profiler results after 10 secs
> > > > > > > > drags.
>
> > > > > > > > In the above pictures I see almost twice (2x) the gain, and
> > > > remember
> > > > > > > > that the test with UI.draggables is the worst case for my patch,
> > > > my
> > > > > > > > patch was mostly aimed at normal events, but seems
> > > > custom/namespaced
> > > > > > > > events will also benefit in some way, or probably the in-between
> > > > > > > > changes that Brandon did also helped...this is the reason I
> > > > updated/
> > > > > > > > reapplied my previous patch to the new SVN. I know there are
> > > > > > > > improvements.
>
> > > > > > > > I will review/retest them tomorrow if I don't get a confirm/deny,
> > > > I am
> > > > > > > > sure me or you (weepy) have different tests. Please download the
> > > > files
> > > > > > > > I used in a tgz pakage here and help me understand:
>
> > > > > > > >  http://javascript.nwbox.com/JQ_TWEAKS2/draggables.tgz
>
> > > > > > > > I have tested that with Firefox 2.0.14 because in that browser I
> > > > we
> > > > > > > > are creating new writable copies of the event object (slow for
> > > > custom/
> > > > > > > > namespaced).
>
> > > > > > > > I an all the other cases the gains are much bigger, and in my view
> > > > > > > > those cases are also more generally known and used (not my tests).
> > > > I
> > > > > > > > love the many jquery plugins (also I admit I don't have time to
> > > > use)
> > > > > > > > and UI is the one that will benefit from this speed improvements.
> > > > > > > > Surely my needs or my view don't say anything about what is
> > > > > > > > "generally" known and used in the web.
>
> > > > > > > > Diego Perini
>
> > > > > > > > On 4 Mag, 00:08, "Brandon Aaron" <brandon.aa...@...> wrote:
> > > > > > > > > That is diego's patch. Feel free to run it against the latest
> > > > SVN to
> > > > > > see
> > > > > > > > how
> > > > > > > > > it stacks up.
>
> > > > > > > > > --
> > > > > > > > > Brandon Aaron
>
> > > > > > > > > On Sat, May 3, 2008 at 4:58 PM, weepy <jonah...@...>
> > > > wrote:
>
> > > > > > > > > > From a simple test on ui.draggables (15seconds of dragging a
> > > > box
> > > > > > > > > > around on each) I get the following :
>
> > > > > > > > > > 1.2.3
> > > > > > > > > > fix     2847    43.73%  886.123ms       909.377ms
> > > > 0.319ms
> > > > > > > > 0.006ms
> > > > > > > > > > 3.674ms event.js
> > > > > > > > > > (line 279)
> > > > > > > > > > drag    1421    12.3%   249.253ms       769.593ms
> > > > 0.542ms
> > > > > > > > 0.397ms
> > > > > > > > > > 1.12ms
> > > > > > > > > > ui.draggable.js (line 195)
> > > > > > > > > > trigger 1423    8.75%   177.38ms        230.843ms
> > > > 0.162ms
> > > > > > > > 0.134ms
> > > > > > > > > > 0.305ms event.js
> > > > > > > > > > (line 161)
>
> > > > > > > > > > Brandons Patch
> > > > > > > > > > create  1400
>
> ...
>
> leggi tutto