Skip to content

Commit

Permalink
Handle NEEDS_MULTIPLE_PASSES more consistently (#73)
Browse files Browse the repository at this point in the history
  • Loading branch information
NebelNidas authored Apr 19, 2024
1 parent 3e56ee2 commit c5fa175
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 66 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Overhauled the internal `ColumnFileReader` to behave more consistently and future-proof
- Overhauled the internal `ColumnFileReader` to behave more consistently
- Made handling of the `NEEDS_MULTIPLE_PASSES` flag more consistent, reducing memory usage in a few cases
- Made some internal methods in Enigma and TSRG readers actually private

## [0.6.1] - 2024-04-15
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

if (parentVisitor != null) {
((MappingTree) visitor).accept(parentVisitor);
}
((MappingTree) visitor).accept(parentVisitor);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,31 +51,40 @@ public static void read(Reader reader, String sourceNs, String targetNs, Mapping
private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
Set<MappingFlag> flags = visitor.getFlags();
MappingVisitor parentVisitor = null;
boolean readerMarked = false;

if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS) || flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS)) {
parentVisitor = visitor;
visitor = new MemoryMappingTree();
} else if (flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

if (visitor.visitHeader()) {
visitor.visitNamespaces(sourceNs, Collections.singletonList(targetNs));
}
for (;;) {
if (visitor.visitHeader()) {
visitor.visitNamespaces(sourceNs, Collections.singletonList(targetNs));
}

if (visitor.visitContent()) {
StringBuilder commentSb = new StringBuilder(200);
final MappingVisitor finalVisitor = visitor;
if (visitor.visitContent()) {
StringBuilder commentSb = new StringBuilder(200);
final MappingVisitor finalVisitor = visitor;

do {
if (reader.nextCol("CLASS")) { // class: CLASS <name-a> [<name-b>]
readClass(reader, 0, null, null, commentSb, finalVisitor);
}
} while (reader.nextLine(0));
}
do {
if (reader.nextCol("CLASS")) { // class: CLASS <name-a> [<name-b>]
readClass(reader, 0, null, null, commentSb, finalVisitor);
}
} while (reader.nextLine(0));
}

if (visitor.visitEnd() && parentVisitor == null) return;
if (visitor.visitEnd()) break;

if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

if (parentVisitor == null) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
int markIdx = reader.reset();
assert markIdx == 1;
}

if (parentVisitor != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ public static void read(Reader reader, String sourceNs, String targetNs, Mapping
private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
Set<MappingFlag> flags = visitor.getFlags();
MappingVisitor parentVisitor = null;
boolean readerMarked = false;

if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS)) {
parentVisitor = visitor;
visitor = new MemoryMappingTree();
} else if (flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

for (;;) {
Expand Down Expand Up @@ -134,7 +136,12 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.visitEnd()) break;

reader.reset();
if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

int markIdx = reader.reset();
assert markIdx == 1;
}

if (parentVisitor != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,15 @@

package net.fabricmc.mappingio.format.proguard;

import java.io.BufferedReader;
import java.io.CharArrayReader;
import java.io.IOException;
import java.io.Reader;
import java.util.Arrays;
import java.util.Collections;

import net.fabricmc.mappingio.MappedElementKind;
import net.fabricmc.mappingio.MappingFlag;
import net.fabricmc.mappingio.MappingUtil;
import net.fabricmc.mappingio.MappingVisitor;
import net.fabricmc.mappingio.format.ColumnFileReader;
import net.fabricmc.mappingio.format.MappingFormat;

/**
Expand All @@ -44,45 +42,33 @@ public static void read(Reader reader, MappingVisitor visitor) throws IOExceptio
}

public static void read(Reader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
BufferedReader br = reader instanceof BufferedReader ? (BufferedReader) reader : new BufferedReader(reader);

read(br, sourceNs, targetNs, visitor);
read(new ColumnFileReader(reader, /* random illegal character */ ';', ' '), sourceNs, targetNs, visitor);
}

private static void read(BufferedReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
CharArrayReader parentReader = null;
private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
boolean readerMarked = false;

if (visitor.getFlags().contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
char[] buffer = new char[100_000];
int pos = 0;
int len;

while ((len = reader.read(buffer, pos, buffer.length - pos)) >= 0) {
pos += len;

if (pos == buffer.length) buffer = Arrays.copyOf(buffer, buffer.length * 2);
}

parentReader = new CharArrayReader(buffer, 0, pos);
reader = new BufferedReader(parentReader);
reader.mark();
readerMarked = true;
}

StringBuilder tmp = null;
StringBuilder descSb = null;

for (;;) {
boolean visitHeader = visitor.visitHeader();

if (visitHeader) {
if (visitor.visitHeader()) {
visitor.visitNamespaces(sourceNs, Collections.singletonList(targetNs));
}

if (visitor.visitContent()) {
if (tmp == null) tmp = new StringBuilder();
if (descSb == null) descSb = new StringBuilder();

String line;
boolean visitClass = false;

while ((line = reader.readLine()) != null) {
do {
if ((line = reader.nextCols(false)) == null) continue;

line = line.trim();
if (line.isEmpty() || line.startsWith("#")) continue;

Expand Down Expand Up @@ -111,7 +97,7 @@ private static void read(BufferedReader reader, String sourceNs, String targetNs

if (parts[1].indexOf('(') < 0) { // field: <type> <deobf> -> <obf>
String name = parts[1];
String desc = pgTypeToAsm(parts[0], tmp);
String desc = pgTypeToAsm(parts[0], descSb);

if (visitor.visitField(name, desc)) {
String mappedName = parts[3];
Expand Down Expand Up @@ -143,7 +129,7 @@ private static void read(BufferedReader reader, String sourceNs, String targetNs
if (part1.lastIndexOf('.', pos - 1) < 0 && part1.length() == pos3 + 1) { // no inlined method
String name = part1.substring(0, pos);
String argDesc = part1.substring(pos, pos3 + 1);
String desc = pgDescToAsm(argDesc, retType, tmp);
String desc = pgDescToAsm(argDesc, retType, descSb);

if (visitor.visitMethod(name, desc)) {
String mappedName = parts[3];
Expand All @@ -153,17 +139,17 @@ private static void read(BufferedReader reader, String sourceNs, String targetNs
}
}
}
}
} while (reader.nextLine(0));
}

if (visitor.visitEnd()) break;

if (parentReader == null) {
if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
} else {
parentReader.reset();
reader = new BufferedReader(parentReader);
}

int markIdx = reader.reset();
assert markIdx == 1;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ public static void read(Reader reader, String sourceNs, String targetNs, Mapping
private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
Set<MappingFlag> flags = visitor.getFlags();
MappingVisitor parentVisitor = null;
boolean readerMarked = false;

if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS)) {
parentVisitor = visitor;
visitor = new MemoryMappingTree();
} else if (flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

for (;;) {
Expand Down Expand Up @@ -129,7 +131,13 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
}

if (visitor.visitEnd()) break;
reader.reset();

if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

int markIdx = reader.reset();
assert markIdx == 1;
}

if (parentVisitor != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,14 @@ public static void read(Reader reader, String sourceNs, String targetNs, Mapping
private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
Set<MappingFlag> flags = visitor.getFlags();
MappingVisitor parentVisitor = null;
boolean readerMarked = false;

if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS)) {
parentVisitor = visitor;
visitor = new MemoryMappingTree();
} else if (flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

for (;;) {
Expand Down Expand Up @@ -159,7 +161,12 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.visitEnd()) break;

reader.reset();
if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

int markIdx = reader.reset();
assert markIdx == 1;
}

if (parentVisitor != null) {
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ public static void read(Reader reader, String sourceNs, String targetNs, Mapping
}

private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException {
MappingFormat format = MappingFormat.SRG_FILE;
Set<MappingFlag> flags = visitor.getFlags();
MappingVisitor parentVisitor = null;
MappingFormat format = MappingFormat.SRG_FILE;
boolean readerMarked = false;

if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS)) {
parentVisitor = visitor;
visitor = new MemoryMappingTree();
} else if (flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

for (;;) {
boolean visitHeader = visitor.visitHeader();

if (visitHeader) {
if (visitor.visitHeader()) {
visitor.visitNamespaces(sourceNs, Collections.singletonList(targetNs));
}

Expand Down Expand Up @@ -155,7 +155,12 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.visitEnd()) break;

reader.reset();
if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

int markIdx = reader.reset();
assert markIdx == 1;
}

if (parentVisitor != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
MappingFormat format = reader.nextCol("tsrg2") ? format = MappingFormat.TSRG_2_FILE : MappingFormat.TSRG_FILE;
String srcNamespace;
List<String> dstNamespaces;
boolean readerMarked = false;

if (format == MappingFormat.TSRG_2_FILE) {
srcNamespace = reader.nextCol();
Expand All @@ -91,6 +92,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.getFlags().contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

int dstNsCount = dstNamespaces.size();
Expand Down Expand Up @@ -178,6 +180,10 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.visitEnd()) break;

if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

int markIdx = reader.reset();
assert markIdx == 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,18 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws
int dstNsCount = dstNamespaces.size();
Set<MappingFlag> flags = visitor.getFlags();
MappingVisitor parentVisitor = null;
boolean readerMarked = false;

if (flags.contains(MappingFlag.NEEDS_ELEMENT_UNIQUENESS) || flags.contains(MappingFlag.NEEDS_HEADER_METADATA)) {
parentVisitor = visitor;
visitor = new MemoryMappingTree();
} else if (flags.contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
reader.mark();
readerMarked = true;
}

for (;;) {
boolean visitHeader = visitor.visitHeader();

if (visitHeader) {
if (visitor.visitHeader()) {
visitor.visitNamespaces(srcNamespace, dstNamespaces);
}

Expand Down Expand Up @@ -170,7 +170,12 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws

if (visitor.visitEnd()) break;

reader.reset();
if (!readerMarked) {
throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
}

int markIdx = reader.reset();
assert markIdx == 1;
}

if (parentVisitor != null) {
Expand Down
Loading

0 comments on commit c5fa175

Please sign in to comment.