From a38c0f5d42af49928bceb30bc305cbce28c0bbe3 Mon Sep 17 00:00:00 2001 From: Radim Dejmek Date: Mon, 21 Sep 2026 07:57:58 +0200 Subject: [PATCH] ERS: sort the hash lists once instead of inserting into a LinkedList by index SortedHashList and SortedIndexedHashList kept their hashes in a LinkedList and found each insertion point by walking it with get(index). LinkedList.get(i) is O(i), so one add was O(n^2) and building a list of n hashes was O(n^3): 10,000 data objects took 347 s in generateTimeStampRequest, rising by about a factor of eight per doubling. Both lists are on the evidence record generation path - SortedIndexedHashList from ERSArchiveTimeStampGenerator.getPartialHashtrees(), SortedHashList from BinaryTreeRootCalculator.computeRootHash() - and neither can be replaced from outside the library, since getPartialHashtrees() is private. Both classes expose only size(), getFirst() and toList(), so collect into an ArrayList and sort once, lazily, in the accessors. The order is unchanged: the old add inserted each hash after every element comparing <= to it, which is where a stable sort of the insertion sequence puts it, and Collections.sort is stable. getFirst() still throws NoSuchElementException on an empty list. Tests: testSortedHashListOrder reproduces the original insertion algorithm and asserts the list matches it element for element over 1,200 pseudo-random values including duplicates and shared prefixes; testLargeDataObjectSet builds a reduced hash tree over 2,000 data objects, reaching both lists, and asserts the root does not depend on the order the objects were added in. Co-Authored-By: Claude Opus 5 --- .../bouncycastle/tsp/ers/SortedHashList.java | 65 ++++---- .../tsp/ers/SortedIndexedHashList.java | 70 +++++---- .../org/bouncycastle/tsp/test/ERSTest.java | 147 ++++++++++++++++++ 3 files changed, 220 insertions(+), 62 deletions(-) diff --git a/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedHashList.java b/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedHashList.java index b571f75106..93f2189974 100644 --- a/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedHashList.java +++ b/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedHashList.java @@ -1,9 +1,10 @@ package org.bouncycastle.tsp.ers; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; -import java.util.LinkedList; import java.util.List; +import java.util.NoSuchElementException; /** * A sorting list - byte[] are sorted in ascending order. @@ -12,7 +13,9 @@ public class SortedHashList { private static final Comparator hashComp = new ByteArrayComparator(); - private final LinkedList baseList = new LinkedList(); + private final List baseList = new ArrayList(); + + private boolean isSorted = true; public SortedHashList() { @@ -20,39 +23,20 @@ public SortedHashList() public byte[] getFirst() { - return (byte[])baseList.getFirst(); + if (baseList.isEmpty()) + { + throw new NoSuchElementException(); + } + + sort(); + + return (byte[])baseList.get(0); } public void add(byte[] hash) { - if (baseList.size() == 0) - { - baseList.addFirst(hash); - } - else - { - if (hashComp.compare(hash, baseList.get(0)) < 0) - { - baseList.addFirst(hash); - } - else - { - int index = 1; - while(index < baseList.size() && hashComp.compare(baseList.get(index), hash) <= 0) - { - index++; - } - - if (index == baseList.size()) - { - baseList.add(hash); - } - else - { - baseList.add(index, hash); - } - } - } + baseList.add(hash); + isSorted = false; } public int size() @@ -62,6 +46,25 @@ public int size() public List toList() { + sort(); + return new ArrayList(baseList); } + + /** + * Sorting is deferred to the accessors. Inserting each hash on add() meant searching a + * LinkedList for the insertion point with get(index), which is O(index), so a single add() + * was O(n^2) and building a list of n hashes was O(n^3). + *

+ * Collections.sort() is stable, so hashes comparing equal keep the order they were added + * in - which is where inserting after the last equal element used to put them. + */ + private void sort() + { + if (!isSorted) + { + Collections.sort(baseList, hashComp); + isSorted = true; + } + } } diff --git a/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedIndexedHashList.java b/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedIndexedHashList.java index c7e875c747..467dc867ef 100644 --- a/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedIndexedHashList.java +++ b/pkix/src/main/java/org/bouncycastle/tsp/ers/SortedIndexedHashList.java @@ -1,9 +1,10 @@ package org.bouncycastle.tsp.ers; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; -import java.util.LinkedList; import java.util.List; +import java.util.NoSuchElementException; /** * A sorting list - byte[] are sorted in ascending order. @@ -12,7 +13,17 @@ public class SortedIndexedHashList { private static final Comparator hashComp = new ByteArrayComparator(); - private final LinkedList baseList = new LinkedList(); + private static final Comparator digestComp = new Comparator() + { + public int compare(IndexedHash l, IndexedHash r) + { + return hashComp.compare(l.digest, r.digest); + } + }; + + private final List baseList = new ArrayList(); + + private boolean isSorted = true; public SortedIndexedHashList() { @@ -20,39 +31,20 @@ public SortedIndexedHashList() public IndexedHash getFirst() { - return (IndexedHash)baseList.getFirst(); + if (baseList.isEmpty()) + { + throw new NoSuchElementException(); + } + + sort(); + + return (IndexedHash)baseList.get(0); } public void add(IndexedHash hash) { - if (baseList.size() == 0) - { - baseList.addFirst(hash); - } - else - { - if (hashComp.compare(hash.digest, ((IndexedHash)baseList.get(0)).digest) < 0) - { - baseList.addFirst(hash); - } - else - { - int index = 1; - while(index < baseList.size() && hashComp.compare(((IndexedHash)baseList.get(index)).digest, hash.digest) <= 0) - { - index++; - } - - if (index == baseList.size()) - { - baseList.add(hash); - } - else - { - baseList.add(index, hash); - } - } - } + baseList.add(hash); + isSorted = false; } public int size() @@ -62,6 +54,22 @@ public int size() public List toList() { + sort(); + return new ArrayList(baseList); } + + /** + * Sorting is deferred to the accessors, for the reason given on SortedHashList.sort(): + * finding the insertion point with LinkedList.get(index) made building a list of n hashes + * O(n^3). Collections.sort() is stable, so hashes comparing equal keep ascending order. + */ + private void sort() + { + if (!isSorted) + { + Collections.sort(baseList, digestComp); + isSorted = true; + } + } } diff --git a/pkix/src/test/java/org/bouncycastle/tsp/test/ERSTest.java b/pkix/src/test/java/org/bouncycastle/tsp/test/ERSTest.java index 051b968a96..f3364f5c63 100644 --- a/pkix/src/test/java/org/bouncycastle/tsp/test/ERSTest.java +++ b/pkix/src/test/java/org/bouncycastle/tsp/test/ERSTest.java @@ -19,7 +19,9 @@ import java.util.Collections; import java.util.Date; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Random; import junit.framework.TestCase; import org.bouncycastle.asn1.ASN1EncodableVector; @@ -68,6 +70,7 @@ import org.bouncycastle.tsp.ers.ERSException; import org.bouncycastle.tsp.ers.ERSFileData; import org.bouncycastle.tsp.ers.ERSInputStreamData; +import org.bouncycastle.tsp.ers.SortedHashList; import org.bouncycastle.util.Arrays; import org.bouncycastle.util.Store; import org.bouncycastle.util.Strings; @@ -1355,6 +1358,150 @@ private int compare(byte[] a, byte[] b) return new BigInteger(1, a).compareTo(new BigInteger(1, b)); } + /** + * SortedHashList used to find each hash's insertion point by walking a LinkedList with + * get(index). The order it produced is the one recorded in every existing evidence record, + * so it is reproduced here from the original algorithm and compared against the list's + * output, over pseudo-random input including duplicates and arrays of differing lengths. + */ + public void testSortedHashListOrder() + { + Random random = new Random(0x5eed); + List input = new ArrayList(); + + for (int i = 0; i != 1000; i++) + { + byte[] value = new byte[random.nextInt(33)]; + random.nextBytes(value); + input.add(value); + } + // duplicates, and values sharing a prefix with a longer one + for (int i = 0; i != 100; i++) + { + input.add((byte[])input.get(i)); + input.add(Arrays.copyOfRange((byte[])input.get(i + 100), 0, ((byte[])input.get(i + 100)).length / 2)); + } + + SortedHashList list = new SortedHashList(); + for (int i = 0; i != input.size(); i++) + { + list.add((byte[])input.get(i)); + } + + List expected = insertionSorted(input); + List actual = list.toList(); + + assertEquals(input.size(), list.size()); + assertEquals(expected.size(), actual.size()); + for (int i = 0; i != expected.size(); i++) + { + assertTrue("differs at " + i, Arrays.areEqual((byte[])expected.get(i), (byte[])actual.get(i))); + } + assertTrue(Arrays.areEqual((byte[])expected.get(0), list.getFirst())); + } + + /** + * The order SortedHashList produced before sorting was deferred to the accessors. + */ + private List insertionSorted(List hashes) + { + LinkedList baseList = new LinkedList(); + + for (int h = 0; h != hashes.size(); h++) + { + byte[] hash = (byte[])hashes.get(h); + + if (baseList.size() == 0) + { + baseList.addFirst(hash); + } + else if (compareUnsigned(hash, (byte[])baseList.get(0)) < 0) + { + baseList.addFirst(hash); + } + else + { + int index = 1; + while (index < baseList.size() && compareUnsigned((byte[])baseList.get(index), hash) <= 0) + { + index++; + } + + if (index == baseList.size()) + { + baseList.add(hash); + } + else + { + baseList.add(index, hash); + } + } + } + + return baseList; + } + + private int compareUnsigned(byte[] left, byte[] right) + { + for (int i = 0; i < left.length && i < right.length; i++) + { + int a = (left[i] & 0xff); + int b = (right[i] & 0xff); + + if (a != b) + { + return a - b; + } + } + return left.length - right.length; + } + + /** + * A reduced hash tree over a large number of data objects. This reaches both sorted lists - + * SortedIndexedHashList from ERSArchiveTimeStampGenerator.getPartialHashtrees(), and + * SortedHashList from BinaryTreeRootCalculator.computeRootHash() - and took about 2.5 + * seconds for these 2,000 objects when the insertion point was found by walking a + * LinkedList, rising by roughly a factor of eight per doubling (10,000 objects took 347 + * seconds). The root is also checked to be independent of the order the objects were added + * in, which is what the sorting is there for. + */ + public void testLargeDataObjectSet() + throws Exception + { + DigestCalculatorProvider digestCalculatorProvider = new JcaDigestCalculatorProviderBuilder().build(); + + List dataObjects = new ArrayList(); + for (int i = 0; i != 2000; i++) + { + dataObjects.add(new ERSByteData(Strings.toByteArray("document " + i))); + } + + byte[] ascending = rootOf(dataObjects, digestCalculatorProvider); + + List reversed = new ArrayList(dataObjects); + Collections.reverse(reversed); + + assertTrue(Arrays.areEqual(ascending, rootOf(reversed, digestCalculatorProvider))); + } + + private byte[] rootOf(List dataObjects, DigestCalculatorProvider digestCalculatorProvider) + throws Exception + { + ERSArchiveTimeStampGenerator ersGen = new ERSArchiveTimeStampGenerator( + digestCalculatorProvider.get(new AlgorithmIdentifier(NISTObjectIdentifiers.id_sha256))); + + for (int i = 0; i != dataObjects.size(); i++) + { + ersGen.addData((ERSData)dataObjects.get(i)); + } + + TimeStampRequestGenerator tspReqGen = new TimeStampRequestGenerator(); + + tspReqGen.setCertReq(true); + + return ersGen.generateTimeStampRequest(tspReqGen).getMessageImprintDigest(); + } + public void testReducedHashTrees() throws Exception {