>My general opinion is that we may choose to compromise code readability a >little only when the optimization is really worth it, especially for public >method >signatures, because otherwise, applying the same logic extensively, we >could end up with a SDK codebase very difficult to read, debug and improve. >This >readability-performance tradeoff may be pushed towards performance when >required in method implementations (e.g. inlining simple method calls, reusing >>objects to reduce memory allocations, etc.)
I am in total agreement with this. That's also what I said in the ticket. FYI, I did more tests to determine the specific improvements of each part: Replacement of split/replace by indexOf to detect simple case: 10% Array recycling: 1% (in the GC) Maurice -----Message d'origine----- De : Cosma Colanicchia [mailto:cosma...@gmail.com] Envoyé : lundi 4 novembre 2013 09:33 À : Apache Flex Developers ML Objet : Re: Need advice on patch validation Thanks to Benoit, I can see the effort put into these patches. However, in this case, I agree in general with the use of indexOf instead of regexp, and to inlining these methods (if it can’t be done by the compiler, see next). The additional array argument could also be ok as we are talking about an Helper class, I see its public methods as an extension to the “helped” class, and not as a part of the “public API”. If this is not the general position, we could consider reaching the same result in some other way? For example, recycling static member array variables in the helper class to avoid reallocations (I think another performance patch related to object deserialization was adopting a similar pattern). Finally, as a side note, I read that Falcon is able to automatically inline method calls, optimizing the byte code at compile time (see [1] and [2] for example, at least it should be implemented in ASC2, don’t known if it also part of the donated Falcon codebase). If this is true, maybe we should give up this kind of manual optimization if we are going to switch to Falcon as the default compiler in the near future, or try to adhere to the requirements for this optimization to happen. (Just my two cents) [1] http://renaun.com/blog/2012/09/using-the-new-inline-metadata-in-asc2/ [2] http://www.bytearray.org/?p=4789 -- Cosma 2013/11/3 Maurice Amsellem <maurice.amsel...@systar.com> > Thanks Justin > ________________________________________ > De : Justin Mclean [jus...@classsoftware.com] > Envoyé : dimanche 3 novembre 2013 22:26 > À : dev@flex.apache.org > Objet : Re: Need advice on patch validation > > Hi, > > > What should be the criteria for accepting an optimization proposition ? > Does it improve significantly improve performance in real use cases for > the majority of users would be at the tip of my list. Calling a method > 10,000 times is sometimes not a real use case. > > > Can we also accept part of an optimization only ? > Of course that's the whole point of submitting patches, commit then review > etc etc > > Thanks, > Justin >
