Skip to content

Commit

Permalink
Use proper logging library (#29)
Browse files Browse the repository at this point in the history 
* Use Log4j for logging

* Address PR feedback

* Switch to SLF4J and Tinylog

* Log more details while in dev-env

* Switch spammy log lines to `trace` level

* Address reviews

* Don't use dedicated logging thread
  • Loading branch information
@NebelNidas
NebelNidas authored Mar 10, 2024
1 parent 9bc6538 commit da3d2b9
Show file tree
Hide file tree
Showing 23 changed files with 226 additions and 186 deletions.
7 changes: 7 additions & 0 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,16 @@ javafx {
dependencies {
api "org.ow2.asm:asm:${asm_version}"
api "org.ow2.asm:asm-tree:${asm_version}"
api "org.slf4j:slf4j-api:${slf4j_version}"
api "net.fabricmc:mapping-io:${mappingio_version}"
implementation "org.ow2.asm:asm-commons:${asm_version}"
implementation "org.ow2.asm:asm-util:${asm_version}"
implementation "com.github.javaparser:javaparser-core:${javaparser_version}"
implementation "net.fabricmc:cfr:${fabric_cfr_version}"
implementation "org.vineflower:vineflower:${vineflower_version}"
implementation "org.bitbucket.mstrobel:procyon-compilertools:${procyon_version}"
runtimeOnly "org.tinylog:tinylog-impl:${tinylog_version}"
runtimeOnly "org.tinylog:slf4j-tinylog:${tinylog_version}"

// JavaFX for all platforms (needed for cross-platform fat jar)
runtimeOnly "org.openjfx:javafx-base:${javafx_version}:win"
Expand Down Expand Up @@ -101,6 +104,10 @@ application {
mainClass = 'matcher.Main'
}

jar {
processResources.exclude('tinylog-dev.properties')
}

publishing {
publications {
mavenJava(MavenPublication) {
Expand Down
2 changes: 2 additions & 0 deletions gradle.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ mappingio_version = 0.5.0
javaparser_version = 3.25.6
javafx_version = 21.0.1
checkstyle_version = 10.12.5
slf4j_version = 2.0.12
tinylog_version = 2.7.0
26 changes: 15 additions & 11 deletions src/main/java/matcher/Matcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import matcher.classifier.ClassClassifier;
import matcher.classifier.ClassifierLevel;
import matcher.classifier.FieldClassifier;
Expand Down Expand Up @@ -173,7 +176,7 @@ public void match(ClassInstance a, ClassInstance b) {
if (a.getArrayDimensions() != b.getArrayDimensions()) throw new IllegalArgumentException("the classes don't have the same amount of array dimensions");
if (a.getMatch() == b) return;

System.out.println("match class "+a+" -> "+b+(a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Matching class {} -> {}{}", a, b, (a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));

if (a.getMatch() != null) {
a.getMatch().setMatch(null);
Expand Down Expand Up @@ -300,7 +303,7 @@ public void match(MethodInstance a, MethodInstance b) {
if (a.getCls().getMatch() != b.getCls()) throw new IllegalArgumentException("the methods don't belong to the same class");
if (a.getMatch() == b) return;

System.out.println("match method "+a+" -> "+b+(a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Matching method {} -> {}{}", a, b, (a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));

Set<MethodInstance> membersA = a.getAllHierarchyMembers();
Set<MethodInstance> membersB = b.getAllHierarchyMembers();
Expand Down Expand Up @@ -369,7 +372,7 @@ public void match(FieldInstance a, FieldInstance b) {
if (a.getCls().getMatch() != b.getCls()) throw new IllegalArgumentException("the methods don't belong to the same class");
if (a.getMatch() == b) return;

System.out.println("match field "+a+" -> "+b+(a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Matching field {} -> {}{}", a, b, (a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));

if (a.getMatch() != null) a.getMatch().setMatch(null);
if (b.getMatch() != null) b.getMatch().setMatch(null);
Expand All @@ -387,7 +390,7 @@ public void match(MethodVarInstance a, MethodVarInstance b) {
if (a.isArg() != b.isArg()) throw new IllegalArgumentException("the method vars are not of the same kind");
if (a.getMatch() == b) return;

System.out.println("match method arg "+a+" -> "+b+(a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Matching method arg {} -> {}{}", a, b, (a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));

if (a.getMatch() != null) a.getMatch().setMatch(null);
if (b.getMatch() != null) b.getMatch().setMatch(null);
Expand All @@ -402,7 +405,7 @@ public void unmatch(ClassInstance cls) {
if (cls == null) throw new NullPointerException("null class");
if (cls.getMatch() == null) return;

System.out.println("unmatch class "+cls+" (was "+cls.getMatch()+")"+(cls.hasMappedName() ? " ("+cls.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Unmatching class {} (was {}){}", cls, cls.getMatch(), (cls.hasMappedName() ? " ("+cls.getName(NameType.MAPPED_PLAIN)+")" : ""));

cls.getMatch().setMatch(null);
cls.setMatch(null);
Expand All @@ -424,7 +427,7 @@ public void unmatch(MemberInstance<?> m) {
if (m == null) throw new NullPointerException("null member");
if (m.getMatch() == null) return;

System.out.println("unmatch member "+m+" (was "+m.getMatch()+")"+(m.hasMappedName() ? " ("+m.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Unmatching member {} (was {}){}", m, m.getMatch(), (m.hasMappedName() ? " ("+m.getName(NameType.MAPPED_PLAIN)+")" : ""));

if (m instanceof MethodInstance) {
for (MethodVarInstance arg : ((MethodInstance) m).getArgs()) {
Expand Down Expand Up @@ -452,7 +455,7 @@ public void unmatch(MethodVarInstance a) {
if (a == null) throw new NullPointerException("null method var");
if (a.getMatch() == null) return;

System.out.println("unmatch method var "+a+" (was "+a.getMatch()+")"+(a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));
LOGGER.debug("Unmatching method var {} (was {}){}", a, a.getMatch(), (a.hasMappedName() ? " ("+a.getName(NameType.MAPPED_PLAIN)+")" : ""));

a.getMatch().setMatch(null);
a.setMatch(null);
Expand Down Expand Up @@ -531,7 +534,7 @@ public boolean autoMatchClasses(ClassifierLevel level, double absThreshold, doub
match(entry.getKey(), entry.getValue());
}

System.out.println("Auto matched "+matches.size()+" classes ("+(classes.size() - matches.size())+" unmatched, "+env.getClassesA().size()+" total)");
LOGGER.info("Auto matched {} classes ({} unmatched, {} total)", matches.size(), (classes.size() - matches.size()), env.getClassesA().size());

return !matches.isEmpty();
}
Expand Down Expand Up @@ -577,7 +580,7 @@ public boolean autoMatchMethods(ClassifierLevel level, double absThreshold, doub
match(entry.getKey(), entry.getValue());
}

System.out.println("Auto matched "+matches.size()+" methods ("+totalUnmatched.get()+" unmatched)");
LOGGER.info("Auto matched {} methods ({} unmatched)", matches.size(), totalUnmatched.get());

return !matches.isEmpty();
}
Expand All @@ -598,7 +601,7 @@ public boolean autoMatchFields(ClassifierLevel level, double absThreshold, doubl
match(entry.getKey(), entry.getValue());
}

System.out.println("Auto matched "+matches.size()+" fields ("+totalUnmatched.get()+" unmatched)");
LOGGER.info("Auto matched {} fields ({} unmatched)", matches.size(), totalUnmatched.get());

return !matches.isEmpty();
}
Expand Down Expand Up @@ -713,7 +716,7 @@ private boolean autoMatchMethodVars(boolean isArg, Function<MethodInstance, Meth
match(entry.getKey(), entry.getValue());
}

System.out.println("Auto matched "+matches.size()+" method "+(isArg ? "arg" : "var")+"s ("+totalUnmatched.get()+" unmatched)");
LOGGER.info("Auto matched {} method {}s ({} unmatched)", matches.size(), (isArg ? "arg" : "var"), totalUnmatched.get());

return !matches.isEmpty();
}
Expand Down Expand Up @@ -843,6 +846,7 @@ public static class MatchingStatus {
}

public static final ExecutorService threadPool = Executors.newWorkStealingPool();
public static final Logger LOGGER = LoggerFactory.getLogger("Matcher");

private final ClassEnvironment env;
private final ClassifierLevel autoMatchLevel = ClassifierLevel.Extra;
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/matcher/Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ public static Handle getTargetHandle(Handle bsm, Object[] bsmArgs) {
} else if (isIrrelevantBsm(bsm)) {
return null;
} else {
System.out.printf("unknown invokedynamic bsm: %s/%s%s (tag=%d iif=%b)%n", bsm.getOwner(), bsm.getName(), bsm.getDesc(), bsm.getTag(), bsm.isInterface());
Matcher.LOGGER.warn("Unknown invokedynamic bsm: {}/{}{} (tag={} iif={})",
bsm.getOwner(), bsm.getName(), bsm.getDesc(), bsm.getTag(), bsm.isInterface());

return null;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/matcher/classifier/ClassClassifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public double getScore(ClassInstance clsA, ClassInstance clsB, ClassEnvironment
@Override
public double getScore(ClassInstance clsA, ClassInstance clsB, ClassEnvironment env) {
/*if (clsA.getName().equals("agl") && clsB.getName().equals("aht")) {
System.out.println();
Matcher.LOGGER.info();
}*/

final double absThreshold = 0.8;
Expand Down
18 changes: 10 additions & 8 deletions src/main/java/matcher/classifier/ClassifierUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.objectweb.asm.util.Textifier;
import org.objectweb.asm.util.TraceMethodVisitor;

import matcher.Matcher;
import matcher.Util;
import matcher.classifier.MatchingCache.CacheToken;
import matcher.type.ClassEnvironment;
Expand Down Expand Up @@ -377,10 +378,11 @@ private static <T> int compareInsns(AbstractInsnNode insnA, AbstractInsnNode ins
implB.getOwner(), implB.getName(), implB.getDesc(), Util.isCallToInterface(implB),
env) ? COMPARED_SIMILAR : COMPARED_DISTINCT;
default:
System.out.println("unexpected impl tag: "+implA.getTag());
Matcher.LOGGER.warn("Unexpected impl tag: {}", implA.getTag());
}
} else if (!Util.isIrrelevantBsm(a.bsm)) {
System.out.printf("unknown invokedynamic bsm: %s/%s%s (tag=%d iif=%b)%n", a.bsm.getOwner(), a.bsm.getName(), a.bsm.getDesc(), a.bsm.getTag(), a.bsm.isInterface());
Matcher.LOGGER.warn("Unknown invokedynamic bsm: {}/{}{} (tag={} iif={})",
a.bsm.getOwner(), a.bsm.getName(), a.bsm.getDesc(), a.bsm.getTag(), a.bsm.isInterface());
}

// TODO: implement
Expand Down Expand Up @@ -622,10 +624,10 @@ private static <T, U> int[] mapLists(T listA, T listB, ListElementRetriever<T, U

/*for (int j = 0; j <= sizeB; j++) {
for (int i = 0; i <= sizeA; i++) {
System.out.printf("%2d ", v[i + j * size]);
Matcher.LOGGER.debug("%2d ", v[i + j * size]);
}
System.out.println();
Matcher.LOGGER.debug("");
}*/

int i = sizeA;
Expand All @@ -641,10 +643,10 @@ private static <T, U> int[] mapLists(T listA, T listB, ListElementRetriever<T, U
if (keepCost <= delCost && keepCost <= insCost) {
if (c - keepCost >= COMPARED_DISTINCT) {
assert c - keepCost == COMPARED_DISTINCT;
//System.out.printf("%d/%d rep %s -> %s%n", i-1, j-1, toString(elementRetriever.apply(listA, i - 1)), toString(elementRetriever.apply(listB, j - 1)));
//Matcher.LOGGER.debug("{}/{} rep {} -> {}", i-1, j-1, toString(elementRetriever.apply(listA, i - 1)), toString(elementRetriever.apply(listB, j - 1)));
ret[i - 1] = -1;
} else {
//System.out.printf("%d/%d eq %s - %s%n", i-1, j-1, toString(elementRetriever.apply(listA, i - 1)), toString(elementRetriever.apply(listB, j - 1)));
//Matcher.LOGGER.debug("{}/{} eq {} - {}", i-1, j-1, toString(elementRetriever.apply(listA, i - 1)), toString(elementRetriever.apply(listB, j - 1)));
ret[i - 1] = j - 1;

/*U e = elementRetriever.apply(listA, i - 1);
Expand All @@ -658,11 +660,11 @@ private static <T, U> int[] mapLists(T listA, T listB, ListElementRetriever<T, U
i--;
j--;
} else if (delCost < insCost) {
//System.out.printf("%d/%d del %s%n", i-1, j-1, toString(elementRetriever.apply(listA, i - 1)));
//Matcher.LOGGER.debug("{}/{} del {}", i-1, j-1, toString(elementRetriever.apply(listA, i - 1)));
ret[i - 1] = -1;
i--;
} else {
//System.out.printf("%d/%d ins %s%n", i-1, j-1, toString(elementRetriever.apply(listB, j - 1)));
//Matcher.LOGGER.debug("{}/{} ins {}", i-1, j-1, toString(elementRetriever.apply(listB, j - 1)));
j--;
}
}
Expand Down
17 changes: 9 additions & 8 deletions src/main/java/matcher/gui/MatchPaneDst.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;

import matcher.Matcher;
import matcher.NameType;
import matcher.classifier.ClassClassifier;
import matcher.classifier.ClassifierLevel;
Expand Down Expand Up @@ -422,7 +423,7 @@ private Boolean evalFilter(List<Object> stack, RankResult<? extends Matchable<?>

for (String part : parts) {
String op = part.toLowerCase(Locale.ENGLISH);
//System.out.printf("stack: %s, op: %s%n", stack, op);
//Matcher.LOGGER.debug("stack: {}, op: {}", stack, op);
byte opTypeA = OP_TYPE_NONE;
byte opTypeB = OP_TYPE_NONE;

Expand Down Expand Up @@ -489,7 +490,7 @@ private Boolean evalFilter(List<Object> stack, RankResult<? extends Matchable<?>
if (type == OP_TYPE_NONE) continue;

if (stack.isEmpty()) {
System.err.println("stack underflow");
Matcher.LOGGER.error("Stack underflow");
return null;
}

Expand All @@ -504,7 +505,7 @@ private Boolean evalFilter(List<Object> stack, RankResult<? extends Matchable<?>
|| type == OP_TYPE_COMPARABLE && operand instanceof Comparable<?>;

if (!valid) {
System.err.println("invalid operand type");
Matcher.LOGGER.debug("Invalid operand type");
return null;
}

Expand All @@ -516,7 +517,7 @@ private Boolean evalFilter(List<Object> stack, RankResult<? extends Matchable<?>
ClassInstance cls = env.getClsByName((String) operand);

if (cls == null) {
System.err.println("unknown class "+operand);
Matcher.LOGGER.debug("Unknown class {}", operand);
return null;
} else {
operand = cls;
Expand All @@ -530,7 +531,7 @@ private Boolean evalFilter(List<Object> stack, RankResult<? extends Matchable<?>
}
}

//System.out.printf("opA: %s, opB: %s%n", opA, opB);
//Matcher.LOGGER.debug("opA: {}, opB: {}", opA, opB);

switch (op) {
case "a":
Expand Down Expand Up @@ -609,16 +610,16 @@ private Boolean evalFilter(List<Object> stack, RankResult<? extends Matchable<?>
}
}

//System.out.printf("res stack: %s%n", stack);
//Matcher.LOGGER.debug("Res stack: {}", stack);

if (stack.isEmpty() || stack.size() > 2) {
System.err.println("no result");
Matcher.LOGGER.info("No result found");
return null;
} else if (stack.size() == 1) {
if (stack.get(0) instanceof Boolean) {
return (Boolean) stack.get(0);
} else {
System.err.println("invalid result");
Matcher.LOGGER.error("Invalid result");
return null;
}
} else { // 2 elements on the stack, use equals
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/matcher/gui/MatchPaneSrc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import javafx.scene.control.TreeView;
import javafx.scene.paint.Color;

import matcher.Matcher;
import matcher.NameType;
import matcher.Util;
import matcher.config.Config;
Expand Down Expand Up @@ -123,7 +124,7 @@ private void retrieveCellTextColors() {
try {
css = parser.parse(Config.getTheme().getUrl().toURI().toURL());
} catch (IOException | URISyntaxException e) {
System.err.println("CSS parsing failed");
Matcher.LOGGER.error("CSS parsing failed", e);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/matcher/gui/menu/UidMenu.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ private void assignMissing() {
}
}

System.out.printf("uids assigned: %d class, %d method, %d field%n",
Matcher.LOGGER.info("UIDs assigned: {} class, {} method, {} field",
nextClassUid - env.nextClassUid,
nextMethodUid - env.nextMethodUid,
nextFieldUid - env.nextFieldUid);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/matcher/gui/tab/WebViewTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ protected void displayHtml(String html) {
html = template.replace("%text%", html)
.replace("%theme_path%", Config.getTheme().getUrl().toExternalForm());

// System.out.println(html);
//Matcher.LOGGER.debug(html);
webView.getEngine().loadContent(html);
}

Expand Down
9 changes: 5 additions & 4 deletions src/main/java/matcher/mapping/MappingPropagator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.Set;
import java.util.function.DoubleConsumer;

import matcher.Matcher;
import matcher.NameType;
import matcher.Util;
import matcher.type.ClassEnvironment;
Expand Down Expand Up @@ -87,7 +88,7 @@ public static boolean propagateNames(ClassEnvironment env, DoubleConsumer progre
}
}

System.out.printf("Propagated %d method names, %d method arg names.", propagatedMethodNames, propagatedArgNames);
Matcher.LOGGER.info("Propagated {} method names and {} method arg names", propagatedMethodNames, propagatedArgNames);

return propagatedMethodNames > 0 || propagatedArgNames > 0;
}
Expand All @@ -114,18 +115,18 @@ public static boolean fixRecordMemberNames(ClassEnvironment env, NameType nameTy

if (linkedMethod.isNameObfuscated()
&& (!field.isNameObfuscated() || !linkedMethod.hasMappedName() || field.hasMappedName())) {
System.out.println("copy record component name for method "+linkedMethod+" from field "+field+" -> "+fieldName);
Matcher.LOGGER.debug("Copying record component name for method {} from field {} -> {}", linkedMethod, field, fieldName);
linkedMethod.setMappedName(fieldName);
} else {
System.out.println("copy record component name for field "+field+" from method "+linkedMethod+" -> "+methodName);
Matcher.LOGGER.debug("Copying record component name for field {} from method {} -> {}", field, linkedMethod, methodName);
field.setMappedName(methodName);
}

modified++;
}
}

System.out.printf("Fixed %d names.%n", modified);
Matcher.LOGGER.info("Fixed {} record names.", modified);

return modified > 0;
}
Expand Down
Loading

0 comments on commit da3d2b9

Please sign in to comment.