Merge pull request #1 from lewysDavies/master

Update fork

Author: PiggyPiglet

README.md

@@ -25,14 +25,13 @@ C: 11.765%  |  11.769%
 ```
 
 # Performance
-Get performance has been significantly improved in comparison to my previous map implementation. This has been achieved with custom compared TreeSets. 
-0.314ms to just 0.004.
+Get performance has been significantly improved in comparison to my previous map implementation. This has been achieved with custom compared TreeSets.
 ```
-Benchmark                Mode  Cnt  Score    Error   Units
-new_collectionAddSingle  avgt   10  0.002 ±  0.001   s/op
-new_collectionGet        avgt   10  0.004 ±  0.001   s/op
-old_mapAddSingle         avgt   10  0.001 ±  0.001   s/op
-old_mapGet               avgt   10  0.314 ±  0.069   s/op
+Benchmark                                 Mode  Cnt      Score     Error  Units
+BenchmarkProbability.collectionAddSingle  avgt    5    501.688 ±  33.925  ns/op
+BenchmarkProbability.collectionGet        avgt    5     69.373 ±   2.198  ns/op
+BenchmarkProbability.mapAddSingle         avgt    5  25809.712 ± 984.980  ns/op
+BenchmarkProbability.mapGet               avgt    5    902.414 ±  22.388  ns/op
 ```
 
 # Installation
@@ -50,7 +49,7 @@ or for the fancy users, you could use Maven:<br>
 <dependency>
     <groupId>com.github.lewysDavies</groupId>
     <artifactId>Java-Probability-Collection</artifactId>
-    <version>v0.6</version>
+    <version>v0.8</version>
 </dependency>
 ```
 **Maven Shade This Dependency:**

pom.xml

@@ -5,11 +5,11 @@
 
 	<groupId>com.lewdev</groupId>
 	<artifactId>probability-lib</artifactId>
-	<version>0.5</version>
+	<version>0.8</version>
 	<packaging>jar</packaging>
 
 	<name>probability-lib</name>
-	<url>http://example.com</url>
+	<url>lewdev.uk</url>
 
 	<properties>
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

src/main/java/com/lewdev/probabilitylib/ProbabilityCollection.java

@@ -1,10 +1,32 @@
+/*
+* Copyright (c) 2020 Lewys Davies
+* 
+* Permission is hereby granted, free of charge, to any person obtaining a copy
+* of this software and associated documentation files (the "Software"), to deal
+* in the Software without restriction, including without limitation the rights
+* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+* copies of the Software, and to permit persons to whom the Software is
+* furnished to do so, subject to the following conditions:
+*
+* The above copyright notice and this permission notice shall be included in all
+* copies or substantial portions of the Software.
+* 
+* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+* SOFTWARE.
+*/
 package com.lewdev.probabilitylib;
 
 import java.util.Comparator;
 import java.util.Iterator;
+import java.util.NavigableSet;
 import java.util.Objects;
+import java.util.SplittableRandom;
 import java.util.TreeSet;
-import java.util.concurrent.ThreadLocalRandom;
 
 /**
  * ProbabilityCollection for retrieving random elements based on probability.
@@ -23,24 +45,22 @@
  * </ul>
  * 
  * @author Lewys Davies
- * @version 0.6
+ * @version 0.8
  *
  * @param <E> Type of elements
  */
 public class ProbabilityCollection<E> {
-
-	protected final Comparator<ProbabilitySetElement<E>> comparator = 
-			(o1, o2)-> Integer.compare(o1.getIndex(), o2.getIndex());
 
-	private final TreeSet<ProbabilitySetElement<E>> collection;
+	private final NavigableSet<ProbabilitySetElement<E>> collection;
+	private final SplittableRandom random = new SplittableRandom();
 
 	private int totalProbability;
 
 	/**
 	 * Construct a new Probability Collection
 	 */
 	public ProbabilityCollection() {
-		this.collection = new TreeSet<>(this.comparator);
+		this.collection = new TreeSet<>(Comparator.comparingInt(ProbabilitySetElement::getIndex));
 		this.totalProbability = 0;
 	}
 
@@ -52,28 +72,28 @@ public int size() {
 	}
 
 	/**
-	 * @return Collection contains no elements
+	 * @return True if collection contains no elements, else False
 	 */
 	public boolean isEmpty() {
 		return this.collection.isEmpty();
 	}
 
 	/**
-	 * @param object
-	 * @return True if the collection contains the object, else False
-	 * @throws IllegalArgumentException if object null
+	 * @param <E> object
+	 * @return True if collection contains the object, else False
+	 * @throws IllegalArgumentException if object is null
 	 */
 	public boolean contains(E object) {
 		if(object == null) {
-			throw new IllegalArgumentException("Cannot check if null object is contained in a collection");
+			throw new IllegalArgumentException("Cannot check if null object is contained in this collection");
 		}
 
 		return this.collection.stream()
 			.anyMatch(entry -> entry.getObject().equals(object));
 	}
 
 	/**
-	 * @return Iterator over collection
+	 * @return Iterator over this collection
 	 */
 	public Iterator<ProbabilitySetElement<E>> iterator() {
 		return this.collection.iterator();
@@ -82,7 +102,7 @@ public Iterator<ProbabilitySetElement<E>> iterator() {
 	/**
 	 * Add an object to this collection
 	 * 
-	 * @param object. Not null.
+	 * @param <E> object. Not null.
 	 * @param probability share. Must be greater than 0.
 	 * 
 	 * @throws IllegalArgumentException if object is null
@@ -97,33 +117,35 @@ public void add(E object, int probability) {
 			throw new IllegalArgumentException("Probability must be greater than 0");
 		}
 
-		this.collection.add(new ProbabilitySetElement<E>(object, probability));
-		this.totalProbability += probability;
+		ProbabilitySetElement<E> entry = new ProbabilitySetElement<E>(object, probability);
+		entry.setIndex(this.totalProbability + 1);
 
-		this.updateIndexes();
+		this.collection.add(entry);
+		this.totalProbability += probability;
 	}
 
 	/**
 	 * Remove a object from this collection
 	 * 
-	 * @param object
+	 * @param <E> object
 	 * @return True if object was removed, else False.
 	 * 
-	 * @throws IllegalArgumentException if object null
+	 * @throws IllegalArgumentException if object is null
 	 */
 	public boolean remove(E object) {
 		if(object == null) {
 			throw new IllegalArgumentException("Cannot remove null object");
 		}
 
 		Iterator<ProbabilitySetElement<E>> it = this.iterator();
-		boolean removed = it.hasNext();
+		boolean removed = false;
 
 		while(it.hasNext()) {
 			ProbabilitySetElement<E> entry = it.next();
 			if(entry.getObject().equals(object)) {
 				this.totalProbability -= entry.getProbability();
 				it.remove();
+				removed = true;
 			}
 		}
 
@@ -132,6 +154,14 @@ public boolean remove(E object) {
 		return removed;
 	}
 
+	/**
+	 * Remove all objects from this collection
+	 */
+	public void clear() {
+		this.collection.clear();
+		this.totalProbability = 0;
+	}
+	
 	/**
 	 * Get a random object from this collection, based on probability.
 	 * 
@@ -141,11 +171,11 @@ public boolean remove(E object) {
 	 */
 	public E get() {
 		if(this.isEmpty()) {
-			throw new IllegalStateException("Cannot get an element out of a empty set");
+			throw new IllegalStateException("Cannot get an object out of a empty collection");
 		}
 
 		ProbabilitySetElement<E> toFind = new ProbabilitySetElement<>(null, 0);
-		toFind.setIndex(ThreadLocalRandom.current().nextInt(1, this.totalProbability + 1));
+		toFind.setIndex(this.random.nextInt(1, this.totalProbability + 1));
 
 		return Objects.requireNonNull(this.collection.floor(toFind).getObject());
 	}
@@ -164,7 +194,7 @@ public final int getTotalProbability() {
 	 * We then only need to store the start index of each element,
 	 * as we make use of the TreeSet#floor
 	 */
-	private void updateIndexes() {
+	private final void updateIndexes() {
 		int previousIndex = 0;
 
 		for(ProbabilitySetElement<E> entry : this.collection) {
@@ -190,7 +220,7 @@ final static class ProbabilitySetElement<T> {
 		private int index;
 
 		/**
-		 * @param object
+		 * @param <T> object
 		 * @param probability
 		 */
 		protected ProbabilitySetElement(T object, int probability) {
@@ -199,7 +229,7 @@ protected ProbabilitySetElement(T object, int probability) {
 		}
 
 		/**
-		 * @return The actual object
+		 * @return <T> The actual object
 		 */
 		public final T getObject() {
 			return this.object;

src/test/java/com/lewdev/probabilitylib/BenchmarkProbability.java

@@ -1,29 +1,27 @@
 package com.lewdev.probabilitylib;
 
-import java.util.HashMap;
-import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
-import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
 import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
 import org.openjdk.jmh.annotations.Scope;
 import org.openjdk.jmh.annotations.Setup;
 import org.openjdk.jmh.annotations.State;
-import org.openjdk.jmh.annotations.Timeout;
-import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.annotations.TearDown;
 import org.openjdk.jmh.infra.Blackhole;
 import org.openjdk.jmh.runner.Runner;
 import org.openjdk.jmh.runner.RunnerException;
 import org.openjdk.jmh.runner.options.Options;
 import org.openjdk.jmh.runner.options.OptionsBuilder;
 
 @BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
 @State(Scope.Benchmark)
-@Warmup(iterations = 5, time = 5)
-@Timeout(time = 25, timeUnit = TimeUnit.SECONDS)
-@Measurement(iterations = 10, time = 2)
+@Fork(value = 2, jvmArgs = {"-Xms2G", "-Xmx2G"})
 public class BenchmarkProbability {
 
 	public static void main(String[] args) throws RunnerException {
@@ -35,52 +33,51 @@ public static void main(String[] args) throws RunnerException {
 		new Runner(opt).run();
 	}
 
-	private final int elements = 10_000;
+	public int elements = 1_000;
 
-	ProbabilityMap<Integer> map = new ProbabilityMap<>();
-	ProbabilityCollection<Integer> collection = new ProbabilityCollection<>();
+	public int toAdd = elements + 1;
+	public int toAddProb = 10;
 
-	private Map<Integer, Integer> addAllTest = new HashMap<>();
+	private ProbabilityMap<Integer> map;
+	private ProbabilityCollection<Integer> collection;
 
-	@Setup
+	@Setup(Level.Iteration)
 	public void setup() {
+		this.map = new ProbabilityMap<>();
+		this.collection = new ProbabilityCollection<>();
+		
 		for(int i = 0; i < elements; i++) {
 			map.add(i, 1);
 			collection.add(i, 1);
 		}
-		
-		for(int i = elements; i < elements * 2; i++) {
-			addAllTest.put(i, 1);
-		}
 	}
 
-	@Benchmark
-	public void mapAddSingle(Blackhole bh) {
-		boolean added = this.map.add(25000, 1);
-		bh.consume(added);
+	@TearDown(Level.Iteration)
+	public void tearDown() {
+		this.map.clear();
+		this.collection.clear();
+		
+		this.map = null;
+		this.collection = null;
 	}
 
 	@Benchmark
-	public void mapAddAll() {
-		map.addAll(addAllTest);
+	public void mapAddSingle() {
+		this.map.add(toAdd, toAddProb);
 	}
 
 	@Benchmark
 	public void collectionAddSingle() {
-		this.collection.add(25000, 1);
+		this.collection.add(toAdd, toAddProb);
 	}
 
 	@Benchmark
 	public void mapGet(Blackhole bh) {
-		for(int i = 0; i < elements * 2; i++) {
-			bh.consume(map.get());
-		}
+		bh.consume(this.map.get());
 	}
 
 	@Benchmark
 	public void collectionGet(Blackhole bh) {
-		for(int i = 0; i < elements * 2; i++) {
-			bh.consume(collection.get());
-		}
+		bh.consume(this.collection.get());
 	}
 }