Hi @Bruno, I’m reading the Pr review you did for Otavio Pr, nice work guys!
I have two questions: 1) Do you build the entire master with tests or do you build 2)do you used a different maven build with test command that the ones public available on TomEE website? On Wed, Dec 5, 2018 at 1:02 PM Otávio Gonçalves de Santana < [email protected]> wrote: > Thank you Bruno, let's wait to a commiter move it forward :) > > On Wed, Dec 5, 2018 at 2:36 PM Bruno Baptista <[email protected]> wrote: > > > Hi All, > > > > I reviewed Otávio's PR and it looks ok to merge. > > > > Cheers > > > > Bruno Baptista > > https://twitter.com/brunobat_ > > > > > > On 04/12/18 16:25, Otávio Gonçalves de Santana wrote: > > > This PR has the Goal to improve performance and save memory on the > > > collections code. To get this goal, we went to different improvements: > > > > > > 1) Use the Constructor in the collection instead of add > > > > > > Set<String> set = new HashSet<>(); > > > set.addAll(Arrays.asList("alpha", "beta", "gamma")); > > > > > > Set<String> set = new HashSet<>(Arrays.asList("alpha", "beta", > "gamma")); > > > > > > These constructs method is replaced with a single call to a > parametrized > > > constructor which simplifies the code. Also for more performant such as > > > HashSet > > > < > > > https://github.com/dmlloyd/openjdk/blob/jdk/jdk/src/java.base/share/classes/java/util/HashSet.java#L119 > > > > > > . > > > > > > 2) Use the addAll instead of add for interactions > > > > > > Replaces add method to calling a bulk method (e.g. > > > collection.addAll(listOfX). This will produce improvements in the code. > > > Such as ArrayList > > > < > > > https://github.com/dmlloyd/openjdk/blob/jdk/jdk/src/java.base/share/classes/java/util/ArrayList.java#L700 > > > > > > and HashMap > > > < > > > https://github.com/dmlloyd/openjdk/blob/jdk/jdk/src/java.base/share/classes/java/util/HashMap.java#L495 > > > > > > > > > 3) Replace Arrays with one element to Collections.singletonList() which > > > will save some memory. > > > > > > 4) Uses EnumSet instead of HashSet > > > > > > From the documentation > > > <https://docs.oracle.com/javase/8/docs/api/java/util/EnumSet.html> > that > > > says: > > > > > > A specialized Set implementation for use with enum types. All of the > > > elements in an enum set must come from a single enum type that is > > > specified, explicitly or implicitly, when the set is created. Enum sets > > are > > > represented internally as bit vectors. This representation is extremely > > > compact and efficient. The space and time performance of this class > > should > > > be good enough to allow its use as a high-quality, typesafe alternative > > to > > > traditional int-based “bit flags.” Even bulk operations (such as > > > containsAll and retainAll) should run very quickly if their argument is > > > also an enum set. > > > > > > 5) Uses the entrySet method: > > > > > > Replaces interaction over the keySet() of a java.util.Map instance, > where > > > the iterated keys are used to retrieve the values from the map. Such > > > iteration may be more efficiently replaced by iteration over the > > entrySet() > > > of the map. The Map.Entry > > > <https://docs.oracle.com/javase/7/docs/api/java/util/Map.Entry.html> > is > > an > > > entry within Map, that avoid the get method several times once the key > > and > > > the value are in the object. > > > Benchmarking > > > > > > Using the JMH <https://openjdk.java.net/projects/code-tools/jmh/>, the > > > OpenJDK benchmarking framework I created the code below: > > > List > > > > > > @Warmup(iterations = 5, time = 1)@Measurement(iterations = 20, time = > > > > > > 1)@Fork(3)@BenchmarkMode(Mode.Throughput)@OutputTimeUnit(TimeUnit.MILLISECONDS)@State(Scope.Thread)public > > > class CollectionBenchmark { > > > > > > private static final List<String> FRUITS = Arrays.asList("banana", > > > "apple", "watermelon", "pineapple", "orange"); > > > > > > @Benchmark > > > public List<String> collectionAddAll() { > > > List<String> fruits = new ArrayList<>(); > > > fruits.addAll(FRUITS); > > > return fruits; > > > } > > > > > > @Benchmark > > > public List<String> collectionAddInteraction() { > > > List<String> fruits = new ArrayList<>(); > > > for (String fruit : FRUITS) { > > > fruits.add(fruit); > > > } > > > return fruits; > > > } > > > > > > @Benchmark > > > public List<String> collectionConstructor() { > > > return new ArrayList<>(FRUITS); > > > } > > > } > > > > > > > > > - CollectionBenchmark.collectionAddInteraction thrpt 60 19311,748 > > ops/ms > > > - CollectionBenchmark.collectionAddAll thrpt 60 27181,810 ops/ms > > (around > > > 40,75% faster) > > > - CollectionBenchmark.collectionConstructor thrpt 60 48734,599 > ops/ms > > > (around 79.29 % faster) > > > > > > Map > > > > > > @Warmup(iterations = 5, time = 1)@Measurement(iterations = 20, time = > > > > > > 1)@Fork(3)@BenchmarkMode(Mode.Throughput)@OutputTimeUnit(TimeUnit.MILLISECONDS)@State(Scope.Thread)public > > > class MapBenchmark { > > > > > > private static final Map<String, Integer> NUMBERS = new > HashMap<>(); > > > > > > static { > > > NUMBERS.put("one", 1); > > > NUMBERS.put("two", 2); > > > NUMBERS.put("one", 3); > > > NUMBERS.put("four", 4); > > > NUMBERS.put("five", 5); > > > NUMBERS.put("six", 6); > > > NUMBERS.put("seven", 7); > > > NUMBERS.put("eight", 8); > > > NUMBERS.put("nine", 9); > > > NUMBERS.put("ten", 10); > > > } > > > > > > @Benchmark > > > public List<Object> entrySet() { > > > List<Object> values = new ArrayList<>(); > > > for (Map.Entry<String, Integer> entry : NUMBERS.entrySet()) { > > > values.add(entry.getKey()); > > > values.add(entry.getValue()); > > > } > > > return values; > > > } > > > > > > @Benchmark > > > public List<Object> key() { > > > List<Object> values = new ArrayList<>(); > > > for (String key : NUMBERS.keySet()) { > > > values.add(key); > > > values.add(NUMBERS.get(key)); > > > } > > > > > > return values; > > > } > > > } > > > > > > > > > - MapBenchmark.key thrpt 60 4571,474 > > > - MapBenchmark.entrySet thrpt 60 5630,745 (around 23.17 % faster) > > > > > > > > > Ref: https://github.com/apache/tomee/pull/235 > > > > > > -- Atentamente: César Hernández Mendoza.
