https://bugzilla.wikimedia.org/show_bug.cgi?id=15402
--- Comment #15 from SharkD <[email protected]> 2008-12-27 08:07:52 UTC --- (In reply to comment #14) > (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; For you to single out a single (and extremely minor) change toward a format that is used 14 other times in the file--and then cite it as "too hard to read"--is completely silly. You're being unreasonable and wasting everyone's time. > > 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). Do you really think that coders here are so underqualified that this will seriously throw them off track? Is the average coder who might be affected by this even likely to have his or her changes committed? This would need to be the case for your comment to be meaningful. It's not like you have to remember each variable throughout the entirety of the code. Rather, they're only significant on a block-per-block basis and can be safely forgotten once a block is completed. Anyone who actually knows what they are doing won't be tripped up by this. > 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. In fact, you're wrong. First of all, in the original code, *two* array lookups are done each time, not just one: ta[id][n] Secondly, I've measured almost a 20% performance increase when not doing the direct array lookups with as little as four variables. So, if you're right about something it's significant, but if you're wrong it's not? That seems a bit biased to me. > 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. Again, this, is silly. It's akin to judging a house to be insufficient because its bricks are small. > 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. You're criteria of code readability is subjective. A multitude of coding techniques are used inconsistently in the code, reflecting the patchy nature of its growth. There is no *one* standard used throughout. Methods you have deemed deficient are already used with considerable frequency, yet you fail to make yourself heard in these cases. > 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. It does in fact seem like you're making stuff up, because you could have mentioned all this way back when I filed my initial report. Instead, you appear to be tacking on more and more conditions that didn't exist beforehand in each written exchange. For someone who claims to be considerate of wasting programmers' time, you are failing to do exactly that. -- 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
