From f0ca9ad28143152aff0592162cdb86f03a872c2b Mon Sep 17 00:00:00 2001 From: Pavel Bludov Date: Sat, 31 Oct 2020 20:14:41 +0300 Subject: [PATCH] Issue #344: add multi-thread support to CheckstyleReportsParser --- .../checkstyle/data/CheckstyleRecord.java | 18 +++- .../github/checkstyle/data/DiffReport.java | 93 ++++++++++++------- .../parser/CheckstyleReportsParser.java | 12 ++- 3 files changed, 85 insertions(+), 38 deletions(-) diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java index b9fb50651..df6bc308d 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java @@ -180,14 +180,22 @@ public String getSimpleCuttedSourceName() { * It is used in a single controlled occasion in the code. * * @param other - * another ChechstyleRecord instance under comparison + * another CheckstyleRecord instance under comparison * with this instance. * @return true if instances are equal. */ - public boolean specificEquals(final CheckstyleRecord other) { - return this.line == other.line && this.column == other.column - && this.source.equals(other.source) - && this.message.equals(other.message); + public int specificCompare(final CheckstyleRecord other) { + int diff = Integer.compare(this.line, other.line); + if (diff == 0) { + diff = Integer.compare(this.column, other.column); + } + if (diff == 0) { + diff = this.source.compareTo(other.source); + } + if (diff == 0) { + diff = this.message.compareTo(other.message); + } + return diff; } } diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java index 57260be0d..fe4dd556f 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -42,13 +43,13 @@ public final class DiffReport { * Container for parsed data, * note it is a TreeMap for memory keeping purposes. */ - private Map> records = + private final Map> records = new TreeMap<>(); /** * Container for statistical data. */ - private Statistics statistics = new Statistics(); + private final Statistics statistics = new Statistics(); /** * Getter for data container. @@ -56,7 +57,7 @@ public final class DiffReport { * @return map containing parsed data. */ public Map> getRecords() { - return records; + return Collections.unmodifiableMap(records); } public Statistics getStatistics() { @@ -76,6 +77,7 @@ public Statistics getStatistics() { public void addRecords(List newRecords, String filename) { if (!newRecords.isEmpty()) { + newRecords.sort(new PositionOrderComparator()); final List popped = records.put(filename, newRecords); if (popped != null) { @@ -85,7 +87,6 @@ public void addRecords(List newRecords, records.remove(filename); } else { - Collections.sort(diff, new PositionOrderComparator()); records.put(filename, diff); } } @@ -93,49 +94,77 @@ public void addRecords(List newRecords, } /** - * Creates difference between 2 lists of records. + * Creates difference between 2 sorted lists of records. * - * @param list1 + * @param firstList * the first list. - * @param list2 + * @param secondList * the second list. * @return the difference list. */ private static List produceDiff( - List list1, List list2) { - final List diff = new ArrayList<>(); - for (CheckstyleRecord rec1 : list1) { - if (!isInList(list2, rec1)) { - diff.add(rec1); - } + List firstList, List secondList) { + final List result; + if (firstList.isEmpty()) { + result = secondList; } - for (CheckstyleRecord rec2 : list2) { - if (!isInList(list1, rec2)) { - diff.add(rec2); - } + else if (secondList.isEmpty()) { + result = firstList; + } + else { + result = produceDiff(firstList.iterator(), secondList.iterator()); } - return diff; + return result; } /** - * Compares the record against list of records. + * Creates difference between 2 non-empty iterators of records. * - * @param list - * of records. - * @param testedRecord - * the record. - * @return true, if has its copy in a list. + * @param firstIterator + * the first iterator. + * @param secondIterator + * the second iterator. + * @return the difference list (always sorted). */ - private static boolean isInList(List list, - CheckstyleRecord testedRecord) { - boolean belongsToList = false; - for (CheckstyleRecord checkstyleRecord : list) { - if (testedRecord.specificEquals(checkstyleRecord)) { - belongsToList = true; - break; + private static List produceDiff( + Iterator firstIterator, Iterator secondIterator) { + CheckstyleRecord firstVal = firstIterator.next(); + CheckstyleRecord secondVal = secondIterator.next(); + final List result = new ArrayList<>(); + while (true) { + final int diff = firstVal.specificCompare(secondVal); + if (diff < 0) { + result.add(firstVal); + if (!firstIterator.hasNext()) { + result.add(secondVal); + break; + } + firstVal = firstIterator.next(); + } + else if (diff > 0) { + result.add(secondVal); + if (!secondIterator.hasNext()) { + result.add(firstVal); + break; + } + secondVal = secondIterator.next(); + } + else { + if (!firstIterator.hasNext() || !secondIterator.hasNext()) { + break; + } + firstVal = firstIterator.next(); + secondVal = secondIterator.next(); } } - return belongsToList; + // add tails + while (firstIterator.hasNext()) { + result.add(firstIterator.next()); + } + while (secondIterator.hasNext()) { + result.add(secondIterator.next()); + } + return result; } /** diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java index c4a1cee38..e0f061a80 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.concurrent.CompletableFuture; import javax.xml.stream.XMLEventReader; import javax.xml.stream.XMLStreamException; @@ -139,6 +140,7 @@ public static DiffReport parse(Path baseXml, Path patchXml, int portionSize) /** * Parses portion of the XML report. + * Difference generation is performed asynchronously for efficient CPU usage. * * @param diffReport * container for parsed data. @@ -157,6 +159,7 @@ private static void parseXmlPortion(DiffReport diffReport, int counter = numOfFilenames; String filename = null; List records = null; + final List> tasks = new ArrayList<>(); while (reader.hasNext()) { final XMLEvent event = reader.nextEvent(); if (event.isStartElement()) { @@ -187,13 +190,20 @@ else if (startElementName.equals(ERROR_TAG)) { if (event.isEndElement()) { final EndElement endElement = event.asEndElement(); if (endElement.getName().getLocalPart().equals(FILE_TAG)) { - diffReport.addRecords(records, filename); + final List currRecords = records; + records = null; + final String currFile = filename; + filename = null; + tasks.add(CompletableFuture.runAsync(() -> { + diffReport.addRecords(currRecords, currFile); + })); if (counter == 0) { break; } } } } + tasks.forEach(CompletableFuture::join); } /**