Skip to content

Prototype improvements to compare performance #332

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Prototype improvements to compare performance #332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 49 additions & 17 deletions src/main/java/org/spdx/utility/compare/SpdxComparer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
import org.spdx.library.model.v2.license.AnyLicenseInfo;
import org.spdx.library.model.v2.license.ExtractedLicenseInfo;
import org.spdx.licenseTemplate.LicenseTextHelper;

import javax.annotation.Nullable;

/**
* Performs a comparison between two or more SPDX documents and holds the results of the comparison
* <p>
Expand Down Expand Up @@ -149,6 +152,8 @@ public class SpdxComparer {
// Snippet references comparison results
private final Map<SpdxDocument, Map<SpdxDocument, List<SpdxSnippet>>> uniqueSnippets = new HashMap<>();
private final Map<String, SpdxSnippetComparer> snippetComparers = new HashMap<>();

private final Map<Integer, Boolean> equivalentElements = new HashMap<>(); // Key is the hash of the hashes of the 2 element

public SpdxComparer() {
// Default empty constructor
Expand Down Expand Up @@ -350,7 +355,7 @@ private void compareExternalDocumentRefs() throws InvalidSPDXAnalysisException {
Collection<ExternalDocumentRef> externalDocRefsB = spdxDocs.get(j).getExternalDocumentRefs();

// find any external refs in A that are not in B
List<ExternalDocumentRef> uniqueA = findUniqueExternalDocumentRefs(externalDocRefsA, externalDocRefsB);
List<ExternalDocumentRef> uniqueA = findUniqueExternalDocumentRefs(externalDocRefsA, externalDocRefsB, equivalentElements);
if (!uniqueA.isEmpty()) {
uniqueAMap.put(spdxDocs.get(j), uniqueA);
}
Expand Down Expand Up @@ -385,7 +390,7 @@ private void compareDocumentRelationships() throws InvalidSPDXAnalysisException
Collection<Relationship> relationshipsB = spdxDocs.get(j).getRelationships();

// find any creators in A that are not in B
List<Relationship> uniqueA = findUniqueRelationships(relationshipsA, relationshipsB);
List<Relationship> uniqueA = findUniqueRelationships(relationshipsA, relationshipsB, equivalentElements);
if (!uniqueA.isEmpty()) {
uniqueAMap.put(spdxDocs.get(j), uniqueA);
}
Expand Down Expand Up @@ -896,7 +901,7 @@ private void compareDocumentContents() throws SpdxCompareException {
Collection<SpdxElement> itemsA = spdxDocs.get(i).getDocumentDescribes();
for (int j = i+1; j < spdxDocs.size(); j++) {
Collection<SpdxElement> itemsB = spdxDocs.get(j).getDocumentDescribes();
if (!collectionsEquivalent(itemsA, itemsB)) {
if (!collectionsEquivalent(itemsA, itemsB, equivalentElements)) {
this.documentContentsEquals = false;
this.differenceFound = true;
return;
Expand Down Expand Up @@ -1156,15 +1161,30 @@ public static boolean objectsEqual(Object o1, Object o2) {
* @throws InvalidSPDXAnalysisException on SPDX parsing errors
*/
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public static boolean elementsEquivalent(Optional<? extends ModelObjectV2> elementA, Optional<? extends ModelObjectV2> elementB) throws InvalidSPDXAnalysisException {
if (elementA.isPresent()) {
if (elementB.isPresent()) {
return elementA.get().equivalent(elementB.get());
public static boolean elementsEquivalent(Optional<? extends ModelObjectV2> elementA,
Optional<? extends ModelObjectV2> elementB,
Map<Integer, Boolean> equivalentElements) throws InvalidSPDXAnalysisException {
return elementsEquivalent(elementA.orElse(null), elementB.orElse(null), equivalentElements);
}

public static boolean elementsEquivalent(@Nullable ModelObjectV2 elementA,
@Nullable ModelObjectV2 elementB,
Map<Integer, Boolean> equivalentElements) throws InvalidSPDXAnalysisException {
if (Objects.nonNull(elementA)) {
if (Objects.nonNull(elementB)) {
int key = Objects.hash(elementA, elementB);
int key2 = Objects.hash(elementA, elementB);
Boolean equiv = equivalentElements.get(key);
if (Objects.isNull(equiv)) {
equiv = elementA.equivalent(elementB);
equivalentElements.put(key, equiv);
}
return equiv;
} else {
return false;
}
} else {
return !elementB.isPresent();
return Objects.isNull(elementB);
}
}

Expand All @@ -1174,7 +1194,9 @@ public static boolean elementsEquivalent(Optional<? extends ModelObjectV2> eleme
* @throws InvalidSPDXAnalysisException on SPDX parsing errors
* @return true if the collections all contain equivalent items
*/
public static boolean collectionsEquivalent(Collection<? extends ModelObjectV2> collectionA, Collection<? extends ModelObjectV2> collectionB) throws InvalidSPDXAnalysisException {
public static boolean collectionsEquivalent(Collection<? extends ModelObjectV2> collectionA,
Collection<? extends ModelObjectV2> collectionB,
Map<Integer, Boolean> equivalentElements) throws InvalidSPDXAnalysisException {
if (Objects.isNull(collectionA)) {
return Objects.isNull(collectionB);
}
Expand All @@ -1184,23 +1206,30 @@ public static boolean collectionsEquivalent(Collection<? extends ModelObjectV2>
if (collectionA.size() != collectionB.size()) {
return false;
}
//DEBUG:
int size = collectionA.size();
int count = 0;
//ENDDEBUG:
for (ModelObjectV2 elementA:collectionA) {

if (Objects.isNull(elementA)) {
continue;
}
boolean found = false;
for (ModelObjectV2 elementB:collectionB) {
if (Objects.isNull(elementB)) {
continue;
}
if (elementA.equivalent(elementB)) {
if (elementsEquivalent(elementA, elementB, equivalentElements)) {
found = true;
break;
}
}
if (!found) {
return false;
}
//DEBUG:
if (size > 10000) {
System.out.println("Compared item "+count++ + "out of "+size);
}
//ENDDEBUG:
}
return true;
}
Expand Down Expand Up @@ -1966,7 +1995,9 @@ public static List<Annotation> findUniqueAnnotations(Collection<Annotation> anno
* @throws InvalidSPDXAnalysisException on SPDX parsing errors
*/
public static List<Relationship> findUniqueRelationships(
Collection<Relationship> relationshipsA, Collection<Relationship> relationshipsB) throws InvalidSPDXAnalysisException {
Collection<Relationship> relationshipsA,
Collection<Relationship> relationshipsB,
Map<Integer, Boolean> equivalentElements) throws InvalidSPDXAnalysisException {
List<Relationship> retval = new ArrayList<>();
if (relationshipsA == null) {
return retval;
Expand All @@ -1978,7 +2009,7 @@ public static List<Relationship> findUniqueRelationships(
boolean found = false;
if (relationshipsB != null) {
for (Relationship relB:relationshipsB) {
if (relA.equivalent(relB)) {
if (elementsEquivalent(relA, relB, equivalentElements)) {
found = true;
break;
}
Expand All @@ -1999,7 +2030,8 @@ public static List<Relationship> findUniqueRelationships(
* @throws InvalidSPDXAnalysisException On error in comparison
*/
public static List<ExternalDocumentRef> findUniqueExternalDocumentRefs(
Collection<ExternalDocumentRef> externalDocRefsA, Collection<ExternalDocumentRef> externalDocRefsB) throws InvalidSPDXAnalysisException {
Collection<ExternalDocumentRef> externalDocRefsA, Collection<ExternalDocumentRef> externalDocRefsB,
Map<Integer, Boolean> equivalentElements) throws InvalidSPDXAnalysisException {
List<ExternalDocumentRef> retval = new ArrayList<>();
if (externalDocRefsA == null) {
return new ArrayList<>();
Expand All @@ -2013,7 +2045,7 @@ public static List<ExternalDocumentRef> findUniqueExternalDocumentRefs(
for (ExternalDocumentRef docRefB:externalDocRefsB) {
if (compareStrings(docRefA.getSpdxDocumentNamespace(),
docRefB.getSpdxDocumentNamespace()) == 0 &&
elementsEquivalent(docRefA.getChecksum(), docRefB.getChecksum())) {
elementsEquivalent(docRefA.getChecksum(), docRefB.getChecksum(), equivalentElements)) {
found = true;
break;
}
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/org/spdx/utility/compare/SpdxItemComparer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,16 @@ public class SpdxItemComparer {
*/
protected Map<SpdxDocument, Map<SpdxDocument, Map<String, String>>> extractedLicenseIdMap;


protected Map<Integer, Boolean> equivalentElements;

public SpdxItemComparer(Map<SpdxDocument, Map<SpdxDocument, Map<String, String>>> extractedLicenseIdMap) {
this(extractedLicenseIdMap, new HashMap<>());
}

public SpdxItemComparer(Map<SpdxDocument, Map<SpdxDocument, Map<String, String>>> extractedLicenseIdMap,
Map<Integer, Boolean> equivalentElements) {
this.extractedLicenseIdMap = extractedLicenseIdMap;
this.equivalentElements = equivalentElements;
}

/**
Expand Down Expand Up @@ -191,13 +198,14 @@ private void compareRelationships(SpdxDocument spdxDocument,
for (Entry<SpdxDocument, SpdxItem> entry : this.documentItem.entrySet()) {
Map<SpdxDocument, List<Relationship>> uniqueCompareRelationship = this.uniqueRelationships.computeIfAbsent(entry.getKey(), k -> new HashMap<>());
Collection<Relationship> compareRelationships = entry.getValue().getRelationships();
List<Relationship> uniqueRelationships = SpdxComparer.findUniqueRelationships(relationships, compareRelationships);
List<Relationship> uniqueRelationships = SpdxComparer.findUniqueRelationships(relationships,
compareRelationships, equivalentElements);
if (!uniqueRelationships.isEmpty()) {
this.relationshipsEquals = false;
this.itemDifferenceFound = true;
}
uniqueDocRelationship.put(entry.getKey(), uniqueRelationships);
uniqueRelationships = SpdxComparer.findUniqueRelationships(compareRelationships, relationships);
uniqueRelationships = SpdxComparer.findUniqueRelationships(compareRelationships, relationships, equivalentElements);
if (!uniqueRelationships.isEmpty()) {
this.relationshipsEquals = false;
this.itemDifferenceFound = true;
Expand Down
67 changes: 4 additions & 63 deletions src/test/java/org/spdx/utility/compare/SpdxComparerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,12 +649,6 @@ private SpdxDocument createTestSpdxDoc(String docUri) throws InvalidSPDXAnalysis
return retval;
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#compare(org.spdx.rdfparser.SpdxDocument, org.spdx.rdfparser.SpdxDocument)}.
* @throws InvalidSPDXAnalysisException
* @throws IOException
* @throws SpdxCompareException
*/
public void testCompare() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -671,13 +665,6 @@ public void testCompare() throws IOException, InvalidSPDXAnalysisException, Spdx
assertEquals(doc2.hashCode(), comparer.getSpdxDoc(1).hashCode());
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#compareLicense(int, org.spdx.rdfparser.license.AnyLicenseInfo, int, org.spdx.rdfparser.license.AnyLicenseInfo)}.
* @throws InvalidSPDXAnalysisException
* @throws IOException
* @throws SpdxCompareException
* @throws InvalidLicenseStringException
*/
public void testCompareLicense() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException, InvalidLicenseStringException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand Down Expand Up @@ -949,12 +936,6 @@ private AnyLicenseInfo updateToNewExtractedLicenseId(AnyLicenseInfo license, Map
}
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isDifferenceFound()}.
* @throws InvalidSPDXAnalysisException
* @throws IOException
* @throws SpdxCompareException
*/
public void testIsDifferenceFound() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -970,12 +951,6 @@ public void testIsDifferenceFound() throws IOException, InvalidSPDXAnalysisExcep

}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isSpdxVersionEqual()}.
* @throws InvalidSPDXAnalysisException
* @throws SpdxCompareException
* @throws IOException
*/
public void testIsSpdxVersionEqual() throws InvalidSPDXAnalysisException, SpdxCompareException, IOException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -993,12 +968,6 @@ public void testIsSpdxVersionEqual() throws InvalidSPDXAnalysisException, SpdxCo
assertFalse(comparer.isDifferenceFound());
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#getSpdxDoc(int)}.
* @throws InvalidSPDXAnalysisException
* @throws IOException
* @throws SpdxCompareException
*/
public void testGetSpdxDoc() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -1013,12 +982,6 @@ public void testGetSpdxDoc() throws IOException, InvalidSPDXAnalysisException, S

}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isDataLicenseEqual()}.
* @throws InvalidSPDXAnalysisException
* @throws IOException
* @throws SpdxCompareException
*/
public void testIsDataLicenseEqual() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -1035,9 +998,6 @@ public void testIsDataLicenseEqual() throws IOException, InvalidSPDXAnalysisExce
assertFalse(comparer.isDataLicenseEqual());
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isDocumentCommentsEqual()}.
*/
public void testIsDocumentCommentsEqual()throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -1055,10 +1015,6 @@ public void testIsDocumentCommentsEqual()throws IOException, InvalidSPDXAnalysis
assertFalse(comparer.isDifferenceFound());
assertTrue(comparer.isDocumentCommentsEqual());
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isExtractedLicensingInfosEqual()}.
*/
public void testIsExtractedLicensingInfosEqual() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand Down Expand Up @@ -1275,9 +1231,6 @@ public void testIsExtractedLicensingInfosEqual() throws IOException, InvalidSPDX
assertTrue(comparer.isDifferenceFound());
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#getUniqueExtractedLicenses(int, int)}.
*/
public void testGetUniqueExtractedLicenses() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand Down Expand Up @@ -1423,9 +1376,6 @@ public void testGetUniqueExtractedLicenses() throws IOException, InvalidSPDXAnal
assertTrue(uniqueLicIds.contains(result.get(3).getLicenseId()));
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#getExtractedLicenseDifferences(int, int)}.
*/
public void testGetExtractedLicenseDifferences() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand Down Expand Up @@ -1667,9 +1617,6 @@ private boolean stringsSame(Collection<String> crossReff1, Collection<String> co
return crossReff1.containsAll(collection);
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isPackageEqual()}.
*/
public void testIsPackageEqual() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand All @@ -1685,9 +1632,6 @@ public void testIsPackageEqual() throws IOException, InvalidSPDXAnalysisExceptio
// note - other test cases will test to make sure isPackageEquals is set for all changes where it should be false
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#isCreatorInformationEqual()}.
*/
public void testIsCreatorInformationEqual() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand Down Expand Up @@ -1745,9 +1689,6 @@ public void testIsCreatorInformationEqual() throws IOException, InvalidSPDXAnaly
assertTrue(comparer.isDifferenceFound());
}

/**
* Test method for {@link org.spdx.compare.SpdxComparer#getUniqueCreators(int, int)}.
*/
public void testGetUniqueCreators() throws IOException, InvalidSPDXAnalysisException, SpdxCompareException {
SpdxComparer comparer = new SpdxComparer();
SpdxDocument doc1 = createTestSpdxDoc(DOC_URIA);
Expand Down Expand Up @@ -2056,10 +1997,10 @@ public void testElementEquivalent() throws InvalidSPDXAnalysisException {
Optional<SpdxElement> element1B = Optional.of(e1b);
Optional<SpdxElement> none1 = Optional.empty();
Optional<SpdxElement> none2 = Optional.empty();
assertTrue(SpdxComparer.elementsEquivalent(element1A, element2A));
assertFalse(SpdxComparer.elementsEquivalent(element1A, element1B));
assertFalse(SpdxComparer.elementsEquivalent(element1A, none1));
assertTrue(SpdxComparer.elementsEquivalent(none1, none2));
assertTrue(SpdxComparer.elementsEquivalent(element1A, element2A, new HashMap<>()));
assertFalse(SpdxComparer.elementsEquivalent(element1A, element1B, new HashMap<>()));
assertFalse(SpdxComparer.elementsEquivalent(element1A, none1, new HashMap<>()));
assertTrue(SpdxComparer.elementsEquivalent(none1, none2, new HashMap<>()));
}

public void testFindUniqueChecksums() throws InvalidSPDXAnalysisException {
Expand Down
Loading