Skip to content

Commit

Permalink
#402: Added TimeOut annotation again to performance test of Immutable…
Browse files Browse the repository at this point in the history
…Object.

Fiddled the test numbers to find a scenario where the non-hash-caching
implementation would still finish within a measurable time. Added a
graph showing the different run numbers (on my computer) for comparison.
The TimeOut is set quite large in comparison to the hash-caching
implementation (2 seconds vs 55 ms), but that is to cover possible
GitHub hickups.

Processed review comments of @rdvdijk.
  • Loading branch information
mvanaken committed Nov 24, 2023
1 parent 5b94155 commit 286d00c
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 13 deletions.
14 changes: 7 additions & 7 deletions core/src/main/java/io/parsingdata/metal/ImmutableObject.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package io.parsingdata.metal;

/**
* When objects are immutable, that means their hashcode will stay the same the moment it is created.
* It will improve performance if these objects cache their hash.
* When objects are immutable, that means their hash code will stay the same the moment it is created.
* It will improve performance if these objects cache their hash code.
* <p>
* This is a lazy implementation, instead of calculating the hash once within the constructor, to avoid
* performance decrease during parsing. In most implementations the hash is not actually used/needed.
* performance decrease during parsing. In most implementations the hash code is not actually used/needed.
*/
public abstract class ImmutableObject {

private Integer hash;
private Integer hashCode;

public abstract int immutableHashCode();

Expand All @@ -18,9 +18,9 @@ public abstract class ImmutableObject {

@Override
public int hashCode() {
if (hash == null) {
hash = immutableHashCode();
if (hashCode == null) {
hashCode = immutableHashCode();
}
return hash;
return hashCode;
}
}
20 changes: 16 additions & 4 deletions core/src/test/java/io/parsingdata/metal/ImmutableObjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;

import io.parsingdata.metal.data.ImmutableList;
import io.parsingdata.metal.data.ParseState;
Expand Down Expand Up @@ -67,11 +69,21 @@ public int immutableHashCode() {
assertEquals(1, counter.get());
}

// Note: This timeout does not stop the test after 1 second.
// The test will run until it finishes and then validate the duration.
@Timeout(value = 2)
@Test
void performanceTest() {
// This test would take way too much time without hash caching.
final int dataBlockCount = 32;
final int dataSize = 64;
// The following graph shows the average duration for dataBlockCount of 6:
//
// dataSize | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 |
// ---------|-----|-----|-----|-----|-----|-----|------|------|
// duration | 0.4 | 0.7 | 1.5 | 3 | 5 | 10 | 18 | 32 | in seconds
//
// Using hash cashing, these are all finished within less then 100 ms.
final int dataBlockCount = 6;
final int dataSize = 10;
final byte[] input = new byte[dataBlockCount*dataSize];
// This token contains recursive tokens to create large ParseGraphs.
final Token deep = repn(
Expand All @@ -91,14 +103,14 @@ void performanceTest() {
assertTrue(result.isPresent());

ImmutableList<ParseValue> allValues = Selection.getAllValues(result.get().order, x -> true);
assertThat(allValues.size, equalTo(2080L));
assertThat(allValues.size, equalTo(66L));

final Map<ParseValue, Value> values = new HashMap<>();
while (allValues != null && allValues.head != null) {
values.put(allValues.head, allValues.head);
allValues = allValues.tail;
}
assertThat(values.size(), equalTo(2080));
assertThat(values.size(), equalTo(66));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private Source createSource() {
@Override protected boolean isAvailable(BigInteger offset, BigInteger length) { return true; }
@Override public int immutableHashCode() { return 0; }
@Override
public boolean equals(Object obj) {return obj == this;}
public boolean equals(Object obj) { return obj == this; }
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class ByTypeTest {
@Override protected boolean isAvailable(BigInteger offset, BigInteger length) { return false; }
@Override public int immutableHashCode() { return -1; }
@Override
public boolean equals(Object obj) {return obj == this;}
public boolean equals(Object obj) { return obj == this; }
};

@Test
Expand Down

0 comments on commit 286d00c

Please sign in to comment.