Skip to content

Commit

Permalink
Issue #344: add multi-thread support to CheckstyleReportsParser
Browse files Browse the repository at this point in the history
  • Loading branch information
Pavel Bludov committed Nov 1, 2020
1 parent fbccbcb commit 611ef49
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package com.github.checkstyle.data;

import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -30,14 +32,21 @@
* @author attatrol
*
*/
public final class CheckstyleRecord {
public final class CheckstyleRecord implements Comparable<CheckstyleRecord> {

/**
* It is usual for sources of records to have name that
* matches this pattern. It is used for shortening source names.
*/
private static final Pattern CHECKSTYLE_CHECK_NAME = Pattern.compile(".+Check");

/**
* Predefined severities for sorting. All other severities has lower priority
* and will be arranged in the default order for strings.
*/
private static final List<String> PREDEFINED_SEVERITIES =
Arrays.asList("info", "warning", "error");

/**
* Length of "Check" string.
*/
Expand Down Expand Up @@ -177,17 +186,67 @@ public String getSimpleCuttedSourceName() {

/**
* Compares CheckstyleRecord instances by their content.
* It is used in a single controlled occasion in the code.
* The order is source, line, column, severity, message.
* Properties index and xref are ignored.
*
* @param other
* another ChechstyleRecord instance under comparison
* another CheckstyleRecord instance under comparison
* with this instance.
* @return true if instances are equal.
* @return 0 if the objects are equal, a negative integer if this record is before the specified
* record, or a positive integer if this record is after the specified record.
*/
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 compareTo(final CheckstyleRecord other) {
int diff = this.source.compareTo(other.source);
if (diff == 0) {
diff = Integer.compare(this.line, other.line);
}
if (diff == 0) {
diff = Integer.compare(this.column, other.column);
}
if (diff == 0) {
diff = compareSeverity(this.severity, other.severity);
}
if (diff == 0) {
diff = this.message.compareTo(other.message);
}
return diff;
}

/**
* Compares record severities in the order "info", "warning", "error", all other.
*
* @param severity1 first severity
* @param severity2 second severity
* @return the value {@code 0} if both severities are the same
* a value less than {@code 0} if severity1 should be first and
* a value greater than {@code 0} if severity2 should be first
*/
private int compareSeverity(String severity1, String severity2) {
final int result;
if (severity1.equals(severity2)) {
result = 0;
}
else {
final int index1 = PREDEFINED_SEVERITIES.indexOf(severity1);
final int index2 = PREDEFINED_SEVERITIES.indexOf(severity2);
if (index1 < 0 && index2 < 0) {
// Both severity levels are unknown, so use regular order for strings.
result = severity1.compareTo(severity2);
}
else if (index1 < 0) {
// First is unknown, second is known: second before
result = 1;
}
else if (index2 < 0) {
// First is known, second is unknown: first before
result = -1;
}
else {
// Both result are well-known, use predefined order
result = Integer.compare(index1, index2);
}
}
return result;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentSkipListMap;

import com.github.checkstyle.parser.CheckstyleReportsParser;

Expand All @@ -42,21 +42,26 @@ public final class DiffReport {
* Container for parsed data,
* note it is a TreeMap for memory keeping purposes.
*/
private Map<String, List<CheckstyleRecord>> records =
new TreeMap<>();
private final Map<String, List<CheckstyleRecord>> records =
new ConcurrentSkipListMap<>();

/**
* Container for statistical data.
*/
private Statistics statistics = new Statistics();
private final Statistics statistics = new Statistics();

/**
* Asynchronous tasks for difference calculation.
*/
private final List<CompletableFuture<Void>> asyncTasks = new ArrayList<>();

/**
* Getter for data container.
*
* @return map containing parsed data.
*/
public Map<String, List<CheckstyleRecord>> getRecords() {
return records;
return Collections.unmodifiableMap(records);
}

public Statistics getStatistics() {
Expand All @@ -76,111 +81,68 @@ public Statistics getStatistics() {
public void addRecords(List<CheckstyleRecord> newRecords,
String filename) {
if (!newRecords.isEmpty()) {
Collections.sort(newRecords);
final List<CheckstyleRecord> popped =
records.put(filename, newRecords);
if (popped != null) {
final List<CheckstyleRecord> diff =
produceDiff(popped, newRecords);
DiffUtils.produceDiff(popped, newRecords);
if (diff.isEmpty()) {
records.remove(filename);
}
else {
Collections.sort(diff, new PositionOrderComparator());
records.put(filename, diff);
}
}
}
}

/**
* Creates difference between 2 lists of records.
* Adds new records to the diff report asynchronously,
* when there are records with this filename, comparison
* between them and new record is performed and only difference is saved.
*
* @param list1
* the first list.
* @param list2
* the second list.
* @return the difference list.
* @param newRecords
* a new records list.
* @param filename
* name of a file which is a cause of records generation.
*/
private static List<CheckstyleRecord> produceDiff(
List<CheckstyleRecord> list1, List<CheckstyleRecord> list2) {
final List<CheckstyleRecord> diff = new ArrayList<>();
for (CheckstyleRecord rec1 : list1) {
if (!isInList(list2, rec1)) {
diff.add(rec1);
}
}
for (CheckstyleRecord rec2 : list2) {
if (!isInList(list1, rec2)) {
diff.add(rec2);
}
}
return diff;
public void addRecordsAsync(
List<CheckstyleRecord> newRecords, String filename) {
asyncTasks.add(CompletableFuture.runAsync(() -> addRecords(newRecords, filename)));
}

/**
* Compares the record against list of records.
*
* @param list
* of records.
* @param testedRecord
* the record.
* @return true, if has its copy in a list.
* Await completion of all asynchronous tasks.
*/
private static boolean isInList(List<CheckstyleRecord> list,
CheckstyleRecord testedRecord) {
boolean belongsToList = false;
for (CheckstyleRecord checkstyleRecord : list) {
if (testedRecord.specificEquals(checkstyleRecord)) {
belongsToList = true;
break;
}
}
return belongsToList;
public void joinAsyncTasksAll() {
asyncTasks.forEach(CompletableFuture::join);
}

/**
* Generates statistical information and puts in in the accumulator.
*/
public void getDiffStatistics() {
statistics.setFileNumDiff(records.size());
for (Map.Entry<String, List<CheckstyleRecord>> entry
: records.entrySet()) {
final List<CheckstyleRecord> list = entry.getValue();
for (CheckstyleRecord rec : list) {
if (rec.getIndex() == CheckstyleReportsParser.BASE_REPORT_INDEX) {
statistics.addSeverityRecordRemoved(rec.getSeverity());
statistics.addModuleRecordRemoved(rec.getSource());
}
else {
statistics.addSeverityRecordAdded(rec.getSeverity());
statistics.addModuleRecordAdded(rec.getSource());
}
statistics.incrementUniqueMessageCount(rec.getIndex());
}
}
records.entrySet().parallelStream()
.flatMap(entry -> entry.getValue().stream())
.forEach(this::addRecordStatistics);
}

/**
* Comparator used to sort lists of CheckstyleRecord objects
* by their position in code.
*
* @author atta_troll
* Generates statistical information for one CheckstyleRecord.
*
* @param checkstyleRecord the checkstyleRecord to process
*/
private static class PositionOrderComparator
implements Comparator<CheckstyleRecord> {

@Override
public int compare(final CheckstyleRecord arg0,
final CheckstyleRecord arg1) {
final int difference = arg0.getLine() - arg1.getLine();
if (difference == 0) {
return arg0.getColumn() - arg1.getColumn();
}
else {
return difference;
}
private void addRecordStatistics(CheckstyleRecord checkstyleRecord) {
if (checkstyleRecord.getIndex() == CheckstyleReportsParser.BASE_REPORT_INDEX) {
statistics.addSeverityRecordRemoved(checkstyleRecord.getSeverity());
statistics.addModuleRecordRemoved(checkstyleRecord.getSource());
}
else {
statistics.addSeverityRecordAdded(checkstyleRecord.getSeverity());
statistics.addModuleRecordAdded(checkstyleRecord.getSource());
}
statistics.incrementUniqueMessageCount(checkstyleRecord.getIndex());
}

}
Loading

0 comments on commit 611ef49

Please sign in to comment.