Hello, Thank you for the response, I was wondering about the volume of refactoring we can do at each component, so let's apply Divide and Conquer approach for each component upgrading work.
I can see a few patterns for the update which I've listed down in my previous mail. We can pick any piece of code and apply focused refactoring on each component, and then we can do it with others as well when we are through with one. It would be a great help if you could suggest a sequence to do so for example : *I --> 4.) Use of single quote for a character *(this is a straightforward work and the easiest one :) ). *II --> **3.) Using Type Inference *(We can pick this as it will never impact any working code). III --> *5.) Updated Variable Declaration.* *IV --> **1.) **Downsize Accessibility Scope *(If tested this is also a not a big deal). *V --> **2.) Using **Lambda Expressions *(This can impact on working hence put at last) This is what I can think :) Please mentor me on this and suggest any better action plan we can opt for. I am very much excited to work on and implement some really cool things that Java Ecosystem can offer us like Functional Programming, Jigsaw <http://openjdk.java.net/projects/jigsaw/>, or Local Variable Type Inference <https://developer.oracle.com/java/jdk-10-local-variable-type-inference>, I am not penning dowing Thanks & Regards, -- *Pradhan Yash Sharma* On Sat, Apr 28, 2018 at 6:59 PM, Taher Alkhateeb <slidingfilame...@gmail.com > wrote: > Hello Yash, > > Thank you for your work on this so far. It's great to see people > focusing on refactoring, which I think should probably be the top > priority for all of us. > > I will review the JIRAs some more over the coming days, but I have a > concern that some of the patches are very large. > > We had many discussions in the past about focused refactoring vs. > general trends. Focused refactoring means you go after a specific > piece of code like a class or group of related classes / artifacts and > fixing them. General trends, on the other hand, means that you > identify a certain pattern and then making a sweeping change across > the entire code base. > > General trends refactorings can be very dangerous, because you are > running after a "trend" not isolating a specific piece of code and > fixing it. > > So my recommendation, especially for the bigger patches that you have, > is to redesign / refactor so that it is a topic-based, not > trend-based. We need to make these commits in isolated bite-sized > chunks that focus on a specific area instead of a specific trend (make > public to private, add try-with-resources, or whatever else) > > Cheers, > Taher > > On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma > <yash.sha...@hotwaxsystems.com> wrote: > > Hi Devs, > > > > Here is the detailed information about the things I am working on for > > performance optimization in our OFBiz code. > > > > *1.) Downsize Accessibility Scope* > > I've tried to downsize accessibility scope of classes, interfaces, > abstract > > class, declared member variables, enumerations, methods, and constructors > > to as minimum as possible as per OFBIz current implementation, still > there > > is a lot of scope for improvement but it would require changes at the > > granular level. I've used this > > <https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html> > as > > my reference point. example: > > > > - public void noteKeyRemoval(UtilCache<K, V> cache, K key, V > oldValue); > > + void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue); > > > > Limiting the scope of the method from public modifier to package level. > > > > *2.) Using Lambda Expressions* > > Then tried to use lambda expressions on simple functional work to > leverage > > implicit type of coding an example: > > > > - Map<String, String> initParameters = new > LinkedHashMap<>(); > > - for (Element e : initParamList) { > > - initParameters.put(e.getAttribute("name"), > > e.getAttribute("value")); > > - } > > + Map<String, String> initParameters = > > initParamList.stream().collect(Collectors.toMap(e -> > > e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b, > > LinkedHashMap::new)); > > > > > > Some of the key benefits of using lambdas will introduce Functional > > style over Imperative style > > <https://stackoverflow.com/questions/2078978/functional- > programming-vs-object-oriented-programming>, > > we can use method referencing > > <https://docs.oracle.com/javase/tutorial/java/javaOO/ > methodreferences.html>, > > usage of aggregate operations, and it will help developers to write > > memory efficient code. > > > > > > *3.) Using Type Inference* > > Java uses type inference so to make code lightweight I've updated > > code constructs as shown in the example for more on this refer this > article > > <https://docs.oracle.com/javase/tutorial/java/generics/ > genTypeInference.htmlv> > > . > > > > - Map<String, ? extends Object> systemProps = > > UtilGenerics.<String, Object> checkMap(System.getProperties()); > > + Map<String, ?> systemProps = > > UtilGenerics.checkMap(System.getProperties()); > > > > > > *4.) Use of single quote for character* > > There is a significant usage of <"Single Character"> in the codebase for > > example: > > > > - throw new GenericConfigException("Error opening file at > > location [" + fileUrl.toExternalForm() + "]", e); > > + throw new GenericConfigException("Error opening file at > > location [" + fileUrl.toExternalForm() + ']', e); > > > > > > "]" is comparativlelly slower then ']' Java internally uses Flyweight > > Design pattern to create String literals so for every call it will not > > create a new Object and used an existing one this will improve > > performace to some extend an study can be seen on this > > <https://stackoverflow.com/questions/24859500/ > concatenate-char-literal-x-vs-single-char-string-literal-x> > > page. > > > > > > *5.) Updated Variable Declaration* > > > > Lastly some of the variable declaration is updated this doesn't create > > a huge difference but helps JVM at the from implicit conversion. > > > > - private long cumulativeEvents = 0; > > + private long cumulativeEvents = 0L; > > > > > > Based on above findings, I have done some code improvement and > > provided following patches. *And need community help for reviewing > > these changes.* > > Kindly provide any improvents or suggestion you have in mind :) > > > > > > 1. [OFBIZ-10344] Refactoring Variable Scope for > > org.apache.ofbiz.base package > > <https://issues.apache.org/jira/browse/OFBIZ-10344> > > 2. [OFBIZ-10345] Refactoring Variable Scope for > > org.apache.ofbiz.catalina.container > > <https://issues.apache.org/jira/browse/OFBIZ-10345> > > 3. [OFBIZ-10346] Refactoring Variable Scope for > > org.apache.ofbiz.common > > <https://issues.apache.org/jira/browse/OFBIZ-10346> > > 4. [OFBIZ-10347] Refactoring Variable Scope for > > org.apache.ofbiz.datafile > > <https://issues.apache.org/jira/browse/OFBIZ-10347> > > 5. [OFBIZ-10348] Refactoring Variable Scope for > > org.apache.ofbiz.entity > > <https://issues.apache.org/jira/browse/OFBIZ-10348> > > > > > > P.S. Apart from this I am also working on performance matrix and will > share > > it soon. > > > > > > Thanks & Regards, > > > > -- > > *Pradhan Yash Sharma* > > > > > > On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma < > yash.sha...@hotwaxsystems.com > >> wrote: > > > >> Thank you for the feedback I've created a Jira ticket OFBIZ-10343 > >> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add > >> patches for the same for your review. > >> > >> Thanks & Regards, > >> -- > >> *Pradhan Yash Sharma* > >> > >> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb < > >> slidingfilame...@gmail.com> wrote: > >> > >>> Hello Pradhan, > >>> > >>> Refactoring is exactly what we need and is a welcomed activity. I > >>> think we should, however, try to avoid "big ideas" across the entire > >>> code base. The subject of your message is the reason why I say that. > >>> > >>> So, if you want to start refactoring, I suggest to start with one > >>> piece of code, study it careful, issue a JIRA, and provide a patch. > >>> This should be focused similar to your notes on UtilCache. > >>> > >>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma > >>> <pradhanyashsharm...@gmail.com> wrote: > >>> > Hello, > >>> > > >>> > While I was working on UtilCache.java file came across some > >>> improvements, > >>> > they are as follows: > >>> > > >>> > 1) Method and Variable access modifiers can be narrowed down to > private > >>> > access modifier. > >>> > > >>> > 2) Then AtomicLong can be given value 0L instead of 0. > >>> > > >>> > 3) Some Variables is used in both synchronized and unsynchronized > >>> blocks, > >>> > so they can be declared final. eg, > >>> > > >>> > > >>> > > >>> > *protected AtomicLong hitCount = new AtomicLong(0); > >>> private > >>> > final AtomicLong hitCount = new AtomicLong(0L);* > >>> > One variable was able to get most of my attention is > >>> > > >>> > * protected ConcurrentMap<Object, CacheLine<V>> > >>> memoryTable > >>> > = null;* > >>> > > >>> > This is used in synchronized and unsynchronized blocks, this Object > can > >>> be > >>> > converted into ThreadLocal or AtomicReference but it would require > >>> changes > >>> > in the current implementation as well. > >>> > > >>> > Lastly, there is extensive use of for loops for iteration we can use > >>> Java 8 > >>> > Streams, Collector, and other functions to leverage implicit looping > >>> > mechanism. > >>> > > >>> > > >>> > -- > >>> > Pradhan Yash Sharma > >>> > >> > >> >
