Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance using Collections in TomEE container #235

Merged
merged 5 commits into from Dec 6, 2018
Merged

Improve performance using Collections in TomEE container #235

merged 5 commits into from Dec 6, 2018

Conversation

otaviojava
Copy link
Contributor

@otaviojava otaviojava commented Dec 4, 2018

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.

  1. 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 and HashMap

  1. Replace Arrays with one element to Collections.singletonList() which will save some memory.

  2. Uses EnumSet instead of HashSet

From the documentation 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.

  1. 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 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, 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)

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
All great improvements, in my opinion.
Code compiles. Running full build in my machine. Will give feedback if something strange pops up.

@brunobat
Copy link
Contributor

brunobat commented Dec 4, 2018

Well... The build failed on the Container Core project, but it was a transient. The build is unstable right now

@otaviojava
Copy link
Contributor Author

otaviojava commented Dec 5, 2018

@brunobat could you try again?
I ran twice with good result:

[INFO] --- maven-install-plugin:2.5.2:install (default-install) @ openejb-junit ---
[INFO] Installing /home/otaviojava/Environment/code/workspace-tomitribe-support/tomee/container/openejb-junit/target/openejb-junit-8.0.0-SNAPSHOT.jar to /home/otaviojava/.m2/repository/org/apache/tomee/openejb-junit/8.0.0-SNAPSHOT/openejb-junit-8.0.0-SNAPSHOT.jar
[INFO] Installing /home/otaviojava/Environment/code/workspace-tomitribe-support/tomee/container/openejb-junit/pom.xml to /home/otaviojava/.m2/repository/org/apache/tomee/openejb-junit/8.0.0-SNAPSHOT/openejb-junit-8.0.0-SNAPSHOT.pom
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for OpenEJB :: Container 8.0.0-SNAPSHOT:
[INFO] 
[INFO] OpenEJB :: Container ............................... SUCCESS [  1.293 s]
[INFO] OpenEJB :: Container :: MBean Annotation API ....... SUCCESS [  1.292 s]
[INFO] OpenEJB :: Container :: API ........................ SUCCESS [  0.424 s]
[INFO] OpenEJB :: Container :: JPA Integration ............ SUCCESS [  0.755 s]
[INFO] OpenEJB :: Container :: Loader ..................... SUCCESS [ 18.419 s]
[INFO] OpenEJB :: Container :: Java Agent ................. SUCCESS [  0.232 s]
[INFO] OpenEJB :: Container :: Java EE .................... SUCCESS [ 16.603 s]
[INFO] OpenEJB :: Container :: Java EE Accessors .......... SUCCESS [  4.636 s]
[INFO] OpenEJB :: Container :: Core ....................... SUCCESS [20:30 min]
[INFO] OpenEJB :: Container :: JUnit ...................... SUCCESS [ 24.689 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  21:39 min
[INFO] Finished at: 2018-12-05T11:25:25-02:00
[INFO] ------------------------------------------------------------------------

@brunobat
Copy link
Contributor

brunobat commented Dec 5, 2018

will do

@brunobat
Copy link
Contributor

brunobat commented Dec 5, 2018

image
Looks ok

@asfgit asfgit merged commit 5c9bad8 into apache:master Dec 6, 2018
@otaviojava otaviojava deleted the improve_collections branch December 7, 2018 09:39
cesarhernandezgt added a commit to cesarhernandezgt/tomee that referenced this pull request Mar 21, 2023
Fix for EarClassLoaderTest and EarWebAppFirstClassLoaderTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants