Skip to content
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.

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

Use IR.STRUCTURE_COMPARATOR to compare two IR objects #11178

Merged
merged 8 commits into from
Sep 30, 2024
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.enso.compiler.core;

import java.util.Comparator;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.function.Function;
Expand Down Expand Up @@ -28,6 +29,23 @@
* <p>See also: Note [IR Equality and hashing]
*/
public interface IR {
/**
* Compares IR structure, but not metadata neither diagnostics. A special comparator used by
* <em>Persistance API</em> to perform some rare consistency checks.
*/
public static final Comparator<IR> STRUCTURE_COMPARATOR =
(aIr, bIr) -> {
if (aIr == bIr) {
return 0;
}
var aCopy = aIr.duplicate(true, false, false, true);
var bCopy = bIr.duplicate(true, false, false, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks expensive if we do it for every IR that we read?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not doing this for every IR we read. The code is only performed before exception:

   sb.append("Adding at ").append(at).append(" object:\n  ");
      dumpObject(sb, res);
      sb.append("\nbut there already is:\n  ");
      dumpObject(sb, prev);
      sb.append("\nare they equal: ").append(bothObjectsAreTheSame);

is logged or thrown. How frequently that happens? It happens on CI from time to time. I haven't seen it on my computer yet.


if (aCopy.equals(bCopy)) {
return 0;
}
return System.identityHashCode(aIr) - System.identityHashCode(bIr);
};

/**
* Storage for metadata that the node has been tagged with as the result of various compiler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.enso.compiler.core.ir.Location;
import org.enso.compiler.core.ir.MetadataStorage;
import org.enso.compiler.core.ir.Module;
import org.enso.compiler.core.ir.Name;
import org.enso.persist.Persistable;
import org.enso.persist.Persistance;
import org.junit.Before;
Expand Down Expand Up @@ -413,6 +414,17 @@ public void writeReplaceReference() throws Exception {
assertEquals("Multiplied on write", 15, (int) plain.get(IntegerSupply.class).supply().get());
}

@Test
public void nameLiteral() throws Exception {
var loc = new IdentifiedLocation(new Location(5, 19), null);
var in =
new Name.Literal("anyName", true, Option.apply(loc), Option.empty(), new MetadataStorage());

var out = serde(Name.Literal.class, in, 50);
assertEquals("They are structuraly equal", 0, IR.STRUCTURE_COMPARATOR.compare(in, out));
assertNotEquals("But not .equals (currently)", in, out);
}

private static <T> T serde(Class<T> clazz, T l, int expectedSize) throws IOException {
return serde(clazz, l, expectedSize, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.io.DataInputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -219,7 +220,7 @@ static Object readIndirect(InputCache cache, PerMap map, Input in) throws IOExce
res = cache.resolveObject(res);
var prev = cache.putObjectAt(at, res);
if (prev != null) {
var bothObjectsAreTheSame = Objects.equals(res, prev);
var bothObjectsAreTheSame = gentlyEquals(res, prev);
var sb = new StringBuilder();
sb.append("Adding at ").append(at).append(" object:\n ");
dumpObject(sb, res);
Expand All @@ -236,6 +237,29 @@ static Object readIndirect(InputCache cache, PerMap map, Input in) throws IOExce
return res;
}

private static Class<?> irClass;
private static Comparator STRUCTURE_COMPARATOR;

@SuppressWarnings("unchecked")
private static boolean gentlyEquals(Object o1, Object o2) {
if (Objects.equals(o1, o2)) {
return true;
}
try {
if (STRUCTURE_COMPARATOR == null) {
irClass = Class.forName("org.enso.compiler.core.IR");
var comparatorField = irClass.getField("STRUCTURE_COMPARATOR");
STRUCTURE_COMPARATOR = (java.util.Comparator) comparatorField.get(null);
}
if (irClass.isInstance(o1) && irClass.isInstance(o2)) {
return STRUCTURE_COMPARATOR.compare(o1, o2) == 0;
}
} catch (ReflectiveOperationException ex) {
PerUtils.LOG.warn("Cannot compare " + o1.getClass(), ex);
}
return false;
}

private static void dumpObject(StringBuilder sb, Object obj) {
sb.append(obj.getClass().getName())
.append("@")
Expand Down
Loading