Handle NEEDS_MULTIPLE_PASSES more consistently

CHANGELOG.md

@@ -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
 
 ## [0.6.1] - 2024-04-15
 - Fixed CSRG and JAM writers sometimes skipping elements whose parents have incomplete destination names

src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaDirReader.java

@@ -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);
 	}
 }

src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java

@@ -51,31 +51,40 @@ public static void read(Reader reader, String sourceNs, String targetNs, Mapping
 	public 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) {

src/main/java/net/fabricmc/mappingio/format/jobf/JobfFileReader.java

@@ -45,12 +45,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 (;;) {
@@ -131,7 +133,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) {

src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileReader.java

@@ -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;
 
 /**
@@ -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;
 
@@ -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];
@@ -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];
@@ -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;
 		}
 	}
 

src/main/java/net/fabricmc/mappingio/format/simple/RecafSimpleFileReader.java

@@ -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 (;;) {
@@ -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) {

src/main/java/net/fabricmc/mappingio/format/srg/JamFileReader.java

@@ -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 (;;) {
@@ -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) {

src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java

@@ -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));
 			}
 
@@ -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) {

src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java

@@ -73,6 +73,7 @@ public static void read(ColumnFileReader reader, String sourceNs, String targetN
 		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();
@@ -91,6 +92,7 @@ public static void read(ColumnFileReader reader, String sourceNs, String targetN
 
 		if (visitor.getFlags().contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) {
 			reader.mark();
+			readerMarked = true;
 		}
 
 		int dstNsCount = dstNamespaces.size();
@@ -178,6 +180,10 @@ public static void read(ColumnFileReader reader, String sourceNs, String targetN
 
 			if (visitor.visitEnd()) break;
 
+			if (!readerMarked) {
+				throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES");
+			}
+
 			int markIdx = reader.reset();
 			assert markIdx == 1;
 		}

src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java

@@ -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);
 			}
 
@@ -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) {