Skip to content

Commit

Permalink
Improve performance of ShortNameComparator.
Browse files Browse the repository at this point in the history
  • Loading branch information
James Cover jdcove2 committed Dec 19, 2024
1 parent 379db5e commit a279775
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 89 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 <= -1) ? s1.length() : nextIndex1) - index1;
final int length2 = ((nextIndex2 <= -1) ? 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;
}

}
102 changes: 27 additions & 75 deletions src/test/java/emissary/util/ShortNameComparatorTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,95 +1,47 @@
package emissary.util;

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 static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;

class ShortNameComparatorTest extends UnitTest {
private final byte[] nobytes = new byte[0];
private static final String b = "foo";
private final String ba = b + Family.SEP;

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

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

@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");
}
import static org.junit.jupiter.api.Assertions.assertThrows;

class ShortNameComparatorTest {
private static final String ABC = "abc";
private static final String DEF = "def";
private static final String PARENT = "foo";
private static final String CHILD_ABC = PARENT + Family.SEP + ABC;
private static final String CHILD_DEF = PARENT + Family.SEP + DEF;
private static final String CHILD_1 = PARENT + Family.SEP + "1";
private static final String CHILD_2 = PARENT + Family.SEP + "2";
private static final String GRANDCHILD_1_1 = CHILD_1 + Family.SEP + "1";

@Test
void testImplComparator() {
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 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 testComparator() {
check(ABC, DEF, 0, 0);
check(CHILD_ABC, CHILD_DEF, -3, 3);
check(CHILD_1, CHILD_2, -1, 1);
check(CHILD_1, GRANDCHILD_1_1, -1, 1);
}

// 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 int reverseResult) {
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(reverseResult, snc.compare(ibdo2, ibdo1));
}
}

0 comments on commit a279775

Please sign in to comment.