https://bugzilla.wikimedia.org/show_bug.cgi?id=15402
--- Comment #14 from Aryeh Gregor <[email protected]> 2008-12-26 15:10:10 UTC --- (In reply to comment #13) > > It mixes things that are probably useful with way too > > much stuff that's ugly (converting if to ternary operator, > > adding temp variables to avoid array accesses) > > Not sure what you're talking about here. There are no instances of if > statements being changed to ternary form. - var ta; - if ( doId ) { - ta = [doId]; - } else { - ta = window.ta; - } + + var ta = doId ? [doId] : window.ta; > Also not sure what you're talking about here. The use of variables to store > common objects accounts for the bulk of the difference in performance. Each instance of this must be separately justified, because every one makes the code harder to read (more lines of code, and you have to remember what more local variables mean). For instance, consider the first one in the file: + var thisTa = ta[id]; + var thisId = thisTa[0]; These will each be referenced a couple of times for each element of the ta array (if that's even defined). But there are also two initializations for each element of the ta array. thisId is initialized and then used *three times*, and thisTa is initialized and then used *once* (not counting the initialization of thisId). This optimization is only even possibly useful if the variable is referred to many times -- I'm quite sure that initializing an extra local variable is much more expensive than an extra array access. If you can demonstrate any detectable performance improvement from this change -- and I mean detectable in normal situations as the code will actually execute, not detectable when you run the code ten million times over in a loop to amplify any differences by a factor of ten million -- I will eat my hat. And my hat is fairly expensive. I am not going to commit any optimization unless either 1) it improves or at least does not harm code readability, or 2) the *exact* optimization is shown to give significant performance improvements. "Significant" means that in some plausible scenario, such as an extremely large sortable table, the difference would actually be noticeable to a normal human being, or at least would come close to being noticeable. And I'm not going to commit 94 lines changed because some of them, somewhere, cause a significant benefit -- you have to show the exact, specific change that causes the benefit, and I'll commit that, not the rest of the stuff that will do nothing. Please note that this is not some stubborn idea of mine. I'm not just refusing to commit this patch because I personally think it's not well done. I'm quite sure that if I committed it, it would be reverted for exactly the reasons I gave -- just because I have commit access doesn't mean that I can commit anything I like. Supposed optimizations that reduce code quality are pretty generally reverted in MediaWiki if they don't give evidence of clear improvement. It doesn't do you or me any good for me to commit things that I believe are bad code. I can't commit anything that I don't believe is up to the project's standards. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
