|
»
»
»
Event.fix optimisation
|
View:
New views
9 Messages
—
Rating Filter:
Alert me
|
| < Prev | 1 - 2 - 3 | Next > |
 |
Re: Event.fix optimisation
by Diego Perini
::
Rate this Message:
Arrix, the link at Mozilla has no explanation either, anyway I remember falling in to a problem with "originalTarget" in which it pointed to a DIV not existing in the DOM. After searching I discovered that "internally" they use a DIV for the element construction and that DIV does not exist in the DOM, and any tentative to access it's properties will yield an error of type "access denied". That's the reason I am concerned about it's use, it can lead developers to use it and then complain, and those kind of problems are hard to track down. We should enforce users to use proper DOM specs and try to avoid to give extra paths if those paths are not offering extra functionality cross-browser. Anyway tank you for being with us, your comments and heads up are strongly appreciated. Diego Perini On 8 Mag, 03:33, Arrix <arrixz...@...> wrote: > Diego, > You are right. "originalTarget" and "explicitOriginalTarget" are Mozilla > specific and should not be copied by jQuery. > I am using jQuey in firefox extension development. A recent update of jQuery > broken some of my old code. Now I've changed my code to use target instead > of originalTarget. > Here is a page comparing event targets:http://developer.mozilla.org/en/docs/DOM:event:Comparison_of_Event_Ta... > > I thought originalTarget was part of DOM2 standard. Sorry! > > > > On Thu, May 8, 2008 at 3:33 AM, Diego Perini <diego.per...@...> wrote: > > > Arrix, > > "originalTarget" and "explicitOriginalTarget" are Mozilla specific > > properties and not part of DOM2 specs that I know. > > > You surely have a valid use for them if you proposed, can you > > show me a simple use case where "target", "currenTarget" and > > "relatedTarget" are not enough ? > > > How can we handle/simulate those capabilities in other browsers > > like IE or Opera so they are usable in practice ? > > > -- > > Diego Perini > > > On 7 Mag, 06:02, Arrix <arrixz...@...> wrote: > > > originalTarget is not copied to the cloned event object but I think it's > > > quite useful. > > > BTW, what properties should be copied? > > > > -- > > > Arrix > > -- > Arrix You received this message because you are subscribed to the Google Groups "jQuery Development" group. To post to this group, send email to jquery-dev@... To unsubscribe from this group, send email to jquery-dev-unsubscribe@... For more options, visit this group at http://groups.google.com/group/jquery-dev?hl=en -~----------~----~----~----~------~----~------~--~--- |
|
|
Re: Event.fix optimisation
by Diego Perini
::
Rate this Message:
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 42.43% 1012.694ms 1012.694ms 0.723ms > > > > 0.425ms > > > > > > 3.765ms > > > > > > jquery.js (line 2097) > > > > > > drag 1398 10.61% 253.316ms 1891.326ms 1.353ms > > > > 0.949ms > > > > > > 4.871ms > > > > > > ui.draggable.js (line 195) > > > > > > handle 2801 9.44% 225.215ms 2320.206ms 0.828ms > > 0.16ms > > > > > > 41.135ms > > > > > > jquery.js (line 2055) > > > > > > > So it seems that whilst 'fix' appears to have been made faster, > > this > > > > > > time has now been displaced into 'create' > > > > > > > Or am I misunderstanding. > > > > > > > weepy > > > > > > > On 3 May, 21:47, Diego Perini <diego.per...@...> wrote: > > > > > > > Brandon, > > > > > > > your annotations make sense and you are correct about > > > > > > > using "jQuery.browser.msie" instead of "createEventObject", > > > > > > > to be consistent with current code. I completely agree. > > > > > > > > You are also correct on the "metaKey" normalization, there > > > > > > > are probably other platform needing that fixed (Mac is one). > > > > > > > > So I will do the changes in my patch and resubmit to the > > > > > > > jquery-dev thread for further inspection by the team. > > > > > > > > If you find other oddities like these or something else I > > > > > > > forgot to include please comment about it and I will > > > > > > > make the modifications in the patch, if you prefer, > > > > > > > modify the original patch as you believe it should > > > > > > > be and send to me the changes for reference. > > > > > > > > Thank you for looking again to the patch and comments > > > > > > > n what is OK and what is not, this way I can fine tune it. > > > > > > > > Diego Perini > > > > > > > > On 3 Mag, 02:07, "Brandon Aaron" <brandon.aa...@...> > > wrote: > > > > > > > > > My biggest problem with the patch still remains to be the > > usage of > > > > > > > > document.createEventObject as the branching logic/feature > > > > detection. > > > > > > That > > > > > > > > might as well be jQuery.browser.msie... The feature detection > > > > _should_ > > > > > > be > > > > > > > > done on the event object itself like found in the current > > > > event.fix. > > > > > > For > > > > > > > > example, we cannot reliably assume that event.which is > > properly > > > > set > > > > > > > > according to the spec in all browsers. The metaKey property > > needs > > > > to > > > > > > be > > > > > > > > normalized in _all_ browsers on a PC. Of course the majority > > of > > > > the > > > > > > > > normalization is due to IE but not all of it and shouldn't be > > > > treated > > > > > > that > > > > > > > > way. > > > > > > > > > Am I making sense here? > > > > > > > > > -- > > > > > > > > Brandon Aaron > > > > > > > > > On Fri, May 2, 2008 at 6:08 PM, Diego Perini < > > > > diego.per...@...> > > > > > > wrote: > > > > > > > > > > Brandon, > > > > > > > > > I have tried to keep all the latest changes and apply my > > patch > > > > > > > > > over the latest SVN r5374. It seems to work for me and I > > have > > > > > > > > > run it through the jQuery test suite on > > Firefox/IE/Opera/Safari > > > > > > > > > on different OS and everything seems smooth. > > > > > > > > > > I am still not using the native browser API to create events > > for > > > > the > > > > > > > > > problems you outlined above, and for some other problem I > > later > > > > > > > > > discovered myself. Anyway your loop through the necessary > > > > > > > > > properties is fast enough, and is now separated in a > > > > > > > > > new "create" method. In this way if we later found > > > > > > > > > a better/faster way to do that the changes should > > > > > > > > > be localized to that method. > > > > > > > > > > I left in a commented method to avoid changing the > > > > "event.target" > > > > > > > > > when events fire on textNode. I simply re-"dispatchEvent()" > > on > > > > the > > > > > > > > > parent node, not sure it really works, can't test that on > > Safari > > > > > > 2.0. > > > > > > > > > > I also have setup the test you sent in the link but > > performance > > > > gain > > > > > > > > > are not perceptible on just a couple of > > "mousemove/mouseover", > > > > > > > > > we should let the test run for some seconds with mouse > > > > interaction. > > > > > > > > > > Anyway the improvements are clear form the three pictures of > > the > > > > > > > > > three profiling tests I have included, everything is linked > > > > here: > > > > > > > > > > http://javascript.nwbox.com/JQ_TWEAKS2/index.html > > > > > > > > > > And this is the direct link to the patched > > > > > > "jquery-r5374-patched.js": > > >http://javascript.nwbox.com/JQ_TWEAKS2/jquery-r5374.patched.js > > > > > > > > > > in case anybody want to download and try that on their code. > > > > > > > > > > As I previously promised, I also worked out a solution for > > the > > > > > > > > > problem Diego A. posed with "jquery.ui.slider" not working, > > this > > > > was > > > > > > > > > due to the namespaced events and it should work now. Tested > > ! > > > > > > > > > > I would like to point out that r5374 does not seem to work > > on > > > > Safari > > > > > > > > > with this same plugin (ui.slider) I tested this while going > > > > through > > > > > > > > > all > > > > > > > > > the previously submitted tests in all browsers I could. > > > > > > > > > > Feel free to correct and comment my > > ... > > read more » > > diegopatched.png > 198KViewDownload > > r5358.png > 193KViewDownload You received this message because you are subscribed to the Google Groups "jQuery Development" group. To post to this group, send email to jquery-dev@... To unsubscribe from this group, send email to jquery-dev-unsubscribe@... For more options, visit this group at http://groups.google.com/group/jquery-dev?hl=en -~----------~----~----~----~------~----~------~--~--- |
|
|
Re: Event.fix optimisation
by Brandon Aaron
::
Rate this Message:
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.perini@...> wrote:
--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "jQuery Development" group. To post to this group, send email to jquery-dev@... To unsubscribe from this group, send email to jquery-dev-unsubscribe@... For more options, visit this group at http://groups.google.com/group/jquery-dev?hl=en -~----------~----~----~----~------~----~------~--~--- |
|
|
Re: Event.fix optimisation
by Diego Perini
::
Rate this Message:
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 42.43% 1012.694ms 1012.694ms > > 0.723ms > > > > > > 0.425ms > > > > > > > > 3.765ms > > > > > > > > jquery.js (line 2097) > > > > > > > > drag 1398 10.61% 253.316ms 1891.326ms > > 1.353ms > > > > > > 0.949ms > > > > > > > > 4.871ms > > > > > > > > ui.draggable.js (line 195) > > > > > > > > handle 2801 9.44% 225.215ms 2320.206ms > > 0.828ms > > > > 0.16ms > > > > > > > > 41.135ms > > > > > > > > jquery.js (line 2055) > > > > > > > > > So it seems that whilst 'fix' appears to have been made > > faster, > > > > this > > > > > > > > time has now been displaced into 'create' > > > > > > > > > Or am I misunderstanding. > > > > > > > > > weepy > > > > > > > > > On 3 May, 21:47, Diego Perini <diego.per...@...> wrote: > > > > > > > > > Brandon, > > > > > > > > > your annotations make sense and you are correct about > > > > > > > > > using "jQuery.browser.msie" instead of "createEventObject", > > > > > > > > > to be consistent with current code. I completely agree. > > > > > > > > > > You are also correct on the "metaKey" normalization, there > > > > > > > > > are probably other platform needing that fixed (Mac is one). > > > > > > > > > > So I will do the changes in my patch and resubmit to the > > > > > > > > > jquery-dev thread for further inspection by the team. > > > > > > > > > > If you find other oddities like these or something else I > > > > > > > > > forgot to include please comment about it and I will > > > > > > > > > make the modifications in the patch, if you prefer, > > > > > > > > > modify the original patch as you believe it should > > > > > > > > > be and send to me the changes for reference. > > > > > > > > > > Thank you for > > ... > > leggi tutto You received this message because you are subscribed to the Google Groups "jQuery Development" group. To post to this group, send email to jquery-dev@... To unsubscribe from this group, send email to jquery-dev-unsubscribe@... For more options, visit this group at http://groups.google.com/group/jquery-dev?hl=en -~----------~----~----~----~------~----~------~--~--- |
|
|
Re: Event.fix optimisation
by Ariel Flesler
::
Rate this Message:
@Diego Forgot to tell you, I closed you merge() ticket last night :) -- Ariel Flesler http://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 42.43% 1012.694ms 1012.694ms > > > 0.723ms > > > > > > > 0.425ms > > > > > > > > > 3.765ms > > > > > > > > > jquery.js (line 2097) > > > > > > > > > drag 1398 10.61% 253.316ms 1891.326ms > > > 1.353ms > > > > > > > 0.949ms > > > > > > > > > 4.871ms > > > > > > > > > ui.draggable.js (line 195) > > > > > > > > > handle 2801 9.44% 225.215ms 2320.206ms > > > 0.828ms > > > > > 0.16ms > > > > > > > > > 41.135ms > > > > > > > > > jquery.js (line 2055) > > > > > > > > > > So it seems that whilst 'fix' appears to have been made > > > faster, > > > > > this > > > > > > > > > time has > > ... > > leer más »- Ocultar texto de la cita - > > - Mostrar texto de la cita - You received this message because you are subscribed to the Google Groups "jQuery Development" group. To post to this group, send email to jquery-dev@... To unsubscribe from this group, send email to jquery-dev-unsubscribe@... For more options, visit this group at http://groups.google.com/group/jquery-dev?hl=en -~----------~----~----~----~------~----~------~--~--- |
|
|
Re: Event.fix optimisation
by Diego Perini
::
Rate this Message:
Ariel, you just got me before I start writing about that too... Thank you for applying the minimal part of it, I will wait for the team to approve the code bifurcation methodology. Sometime, particularly in this case the code size is not growing because there was already a browser bifurcation. If we where allowed to write strings of code and then use "new Function()" we will have a double gain, both in code size and speed, however we will have to wait and see what's AIR move on that. Again, thank you for being so meticulous and squeezing out every milliseconds fat in the core. -- Diego Perini On 13 Mag, 18:01, Ariel Flesler <afles...@...> wrote: > @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 |
