Skip to content

Commit

Permalink
improve performance of ShortNameComparator. (#1027)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdcove2
jdcove2 authored Dec 23, 2024
1 parent 43a112d commit e69fde9
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 81 deletions.
54 changes: 40 additions & 14 deletions src/main/java/emissary/util/ShortNameComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import emissary.core.Family;
import emissary.core.IBaseDataObject;

import org.apache.commons.lang3.Validate;

import java.io.Serializable;
import java.util.Comparator;

Expand All @@ -17,36 +19,60 @@ public class ShortNameComparator implements Comparator<IBaseDataObject>, Seriali

@Override
public int compare(IBaseDataObject obj1, IBaseDataObject obj2) {
Validate.isTrue(obj1 != null, "Required: obj1 != null");
Validate.isTrue(obj2 != null, "Required: obj2 != true");

String s1 = obj1.shortName();
String s2 = obj2.shortName();
int index1 = s1.indexOf(Family.SEP);
int index2 = s2.indexOf(Family.SEP);

String[] p1 = s1.split(Family.SEP);
String[] p2 = s2.split(Family.SEP);

for (int i = 1; i < p1.length; i++) {

if (i >= p2.length) {
// Loop through each level in obj1's shortName.
while (index1 != -1) {
// If obj1's shortName has more levels than obj2's shortName then obj1 > obj2.
if (index2 == -1) {
return 1;
}

if (p1[i].equals(p2[i])) {
// Get start character of level.
index1 += Family.SEP.length();
index2 += Family.SEP.length();

// Get character length of level.
final int nextIndex1 = s1.indexOf(Family.SEP, index1);
final int nextIndex2 = s2.indexOf(Family.SEP, index2);
final int length1 = ((nextIndex1 < 0) ? s1.length() : nextIndex1) - index1;
final int length2 = ((nextIndex2 < 0) ? s2.length() : nextIndex2) - index2;

// If the obj1's level and obj2's level are the same, then go to the next level.
if (length1 == length2 && s1.regionMatches(index1, s2, index2, length1)) {
index1 = nextIndex1;
index2 = nextIndex2;

continue;
}

// Otherwise, try comparing the levels as integers.
try {
int i1 = Integer.parseInt(p1[i]);
int i2 = Integer.parseInt(p2[i]);
return i1 - i2;
int int1 = Integer.parseInt(s1, index1, index1 + length1, 10);
int int2 = Integer.parseInt(s2, index2, index2 + length2, 10);

return int1 - int2;
} catch (NumberFormatException e) {
return p1[i].compareTo(p2[i]);
}
// Otherwise, compare the levels as strings.
final String substring1 = s1.substring(index1, index1 + length1);
final String substring2 = s2.substring(index2, index2 + length2);

return substring1.compareTo(substring2);
}
}

if (p2.length > p1.length) {
// If obj1's shortName has fewer levels than obj2's shortName then obj1 < obj2.
if (index2 != -1) {
return -1;
}

// Otherwise, obj1 and obj2 compare as equivalent
return 0;
}

}
129 changes: 62 additions & 67 deletions src/test/java/emissary/util/ShortNameComparatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,94 +2,89 @@

import emissary.core.BaseDataObject;
import emissary.core.DataObjectFactory;
import emissary.core.Family;
import emissary.core.IBaseDataObject;
import emissary.test.core.junit5.UnitTest;

import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

class ShortNameComparatorTest extends UnitTest {
private final byte[] nobytes = new byte[0];
private static final String b = "foo";
private final String ba = b + Family.SEP;
// @formatter: off
private static final String ABC = "abc";
private static final String DEF = "def";
private static final String FOO_ATT_ABC = "foo-att-abc";
private static final String FOO_ATT_DEF = "foo-att-def";
private static final String FOO_ATT_1 = "foo-att-1";
private static final String FOO_ATT_2 = "foo-att-2";
private static final String FOO_ATT_1_ATT_1 = "foo-att-1-att-1";
private static final String FOO_ATT_1_ATT_2 = "foo-att-1-att-2";
private static final String FOO_ATT_10 = "foo-att-10";
private static final String FOO_ATT_10_ATT_1 = "foo-att-10-att-1";
// @formatter: on
private static final ShortNameComparator COMPARATOR = new ShortNameComparator();

@Test
void testOrdering() {
final List<IBaseDataObject> l = new ArrayList<>();
void testParameters() {
final ShortNameComparator comparator = new ShortNameComparator();
final IBaseDataObject ibdo = new BaseDataObject();

fillList(l);
l.sort(new ShortNameComparator());
checkList(l);
assertThrows(IllegalArgumentException.class, () -> comparator.compare(null, ibdo));
assertThrows(IllegalArgumentException.class, () -> comparator.compare(ibdo, null));
}

@Test
void testNonsenseNames() {
final List<IBaseDataObject> l = new ArrayList<>();
l.add(DataObjectFactory.getInstance(null, "foo-att-def"));
l.add(DataObjectFactory.getInstance(null, "foo-att-abc"));
l.sort(new ShortNameComparator());
assertEquals("foo-att-abc", l.get(0).shortName(), "Bad components sort in alpha order");
}

private void fillList(final List<IBaseDataObject> l) {
l.add(DataObjectFactory.getInstance(this.nobytes, this.ba + "1"));
l.add(DataObjectFactory.getInstance(this.nobytes, this.ba + "3"));
l.add(DataObjectFactory.getInstance(this.nobytes, b));
l.add(DataObjectFactory.getInstance(this.nobytes, this.ba + "3" + Family.getSep(2)));
l.add(DataObjectFactory.getInstance(this.nobytes, this.ba + "3" + Family.getSep(1)));
l.add(DataObjectFactory.getInstance(this.nobytes, this.ba + "2"));
l.add(DataObjectFactory.getInstance(this.nobytes, this.ba + "3" + Family.getSep(1) + Family.getSep(1)));
}

private void checkList(final List<IBaseDataObject> l) {
assertEquals(b, l.get(0).shortName(), "Ordering of sort");
assertEquals(this.ba + "1", l.get(1).shortName(), "Ordering of sort");
assertEquals(this.ba + "2", l.get(2).shortName(), "Ordering of sort");
assertEquals(this.ba + "3", l.get(3).shortName(), "Ordering of sort");
assertEquals(this.ba + "3" + Family.getSep(1), l.get(4).shortName(), "Ordering of sort");
assertEquals(this.ba + "3" + Family.getSep(1) + Family.getSep(1), l.get(5).shortName(), "Ordering of sort");
assertEquals(this.ba + "3" + Family.getSep(2), l.get(6).shortName(), "Ordering of sort");
void testComparator() {
check(ABC, DEF, 0);
check(FOO_ATT_ABC, FOO_ATT_DEF, -3);
check(FOO_ATT_1, FOO_ATT_2, -1);
check(FOO_ATT_1, FOO_ATT_10, -9);
check(FOO_ATT_1, FOO_ATT_1_ATT_1, -1);
}

@Test
void testImplComparator() {
final List<IBaseDataObject> l = new ArrayList<>();

fillList(l);
l.sort(new ShortNameComparator());
checkList(l);
}

@Test
void testSubclassedComparator() {
final String defaultPayloadClass = DataObjectFactory.getImplementingClass();
DataObjectFactory.setImplementingClass(MyDataObject.class.getName());
try {
final List<IBaseDataObject> l = new ArrayList<>();

fillList(l);
l.sort(new ShortNameComparator());
checkList(l);
} catch (RuntimeException ex) {
fail("Cannot operate Comparator in subclass", ex);
} finally {
DataObjectFactory.setImplementingClass(defaultPayloadClass);
}
void testListSorting() {
// start with an unsorted list
List<String> unsorted = List.of(
FOO_ATT_1,
FOO_ATT_10,
FOO_ATT_2,
FOO_ATT_10_ATT_1,
FOO_ATT_1_ATT_2,
FOO_ATT_1_ATT_1);

// map to a list of IBDO, sort using Comparator, map back to a list of Strings
List<String> actual = unsorted
.stream()
.map(name -> DataObjectFactory.getInstance(new byte[0], name))
.sorted(COMPARATOR)
.map(IBaseDataObject::shortName)
.collect(Collectors.toList());

// list with shortnames in expected sort order
List<String> expected = List.of(
FOO_ATT_1,
FOO_ATT_1_ATT_1,
FOO_ATT_1_ATT_2,
FOO_ATT_2,
FOO_ATT_10,
FOO_ATT_10_ATT_1);

// verify that the sorted output is correct
assertIterableEquals(expected, actual, "List wasn't sorted in the expected order");
}

// An extension of BaseDataObject for testing the
// lower bounded generics on the comparator
public static class MyDataObject extends BaseDataObject {
static final long serialVersionUID = 7872122417333007868L;
private static void check(final String name1, final String name2, final int forwardResult) {
final IBaseDataObject ibdo1 = new BaseDataObject(null, name1);
final IBaseDataObject ibdo2 = new BaseDataObject(null, name2);
final ShortNameComparator snc = new ShortNameComparator();

public MyDataObject(final byte[] data, final String name) {
super(data, name);
}
assertEquals(forwardResult, snc.compare(ibdo1, ibdo2));
assertEquals(-forwardResult, snc.compare(ibdo2, ibdo1));
}
}

0 comments on commit e69fde9

Please sign in to comment.