From 8789d7f40c7e4d35055b5faa725e5bf59bfd5cee Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Sun, 3 Jan 2021 23:22:27 +0100 Subject: [PATCH 1/6] BinarySerializer: implement Closeable, Flushable --- src.java/crosby/binary/BinarySerializer.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src.java/crosby/binary/BinarySerializer.java b/src.java/crosby/binary/BinarySerializer.java index 153ea7f..683c2b1 100644 --- a/src.java/crosby/binary/BinarySerializer.java +++ b/src.java/crosby/binary/BinarySerializer.java @@ -17,6 +17,8 @@ package crosby.binary; +import java.io.Closeable; +import java.io.Flushable; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -35,7 +37,7 @@ import crosby.binary.file.FileBlock; * ordered to process their data at the appropriate time. * */ -public class BinarySerializer { +public class BinarySerializer implements Closeable, Flushable { /** * Interface used to write a group of primitives. One of these for each @@ -92,11 +94,13 @@ public class BinarySerializer { return stringtable; } + @Override public void flush() throws IOException { processBatch(); output.flush(); } + @Override public void close() throws IOException { flush(); output.close(); From 22092218afe2e555b5e6cb9c55eae5e2973adf50 Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Sun, 3 Jan 2021 23:27:41 +0100 Subject: [PATCH 2/6] BlockInputStream: implement Closeable --- src.java/crosby/binary/file/BlockInputStream.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src.java/crosby/binary/file/BlockInputStream.java b/src.java/crosby/binary/file/BlockInputStream.java index 1266e5e..1da99bc 100644 --- a/src.java/crosby/binary/file/BlockInputStream.java +++ b/src.java/crosby/binary/file/BlockInputStream.java @@ -17,11 +17,12 @@ package crosby.binary.file; +import java.io.Closeable; import java.io.EOFException; import java.io.IOException; import java.io.InputStream; -public class BlockInputStream { +public class BlockInputStream implements Closeable { // TODO: Should be seekable input stream! public BlockInputStream(InputStream input, BlockReaderAdapter adaptor) { this.input = input; @@ -38,6 +39,7 @@ public class BlockInputStream { } } + @Override public void close() throws IOException { input.close(); } From b787a1fdaa832617f786d7f2007eaed3a7fa5eaa Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Sun, 3 Jan 2021 23:39:01 +0100 Subject: [PATCH 3/6] Modernise Java codebase (Java 8 features) --- src.java/crosby/binary/BinaryParser.java | 2 +- src.java/crosby/binary/BinarySerializer.java | 4 +-- src.java/crosby/binary/StringTable.java | 27 +++++++------------ .../crosby/binary/file/BlockOutputStream.java | 2 +- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src.java/crosby/binary/BinaryParser.java b/src.java/crosby/binary/BinaryParser.java index 557c575..f32d41e 100644 --- a/src.java/crosby/binary/BinaryParser.java +++ b/src.java/crosby/binary/BinaryParser.java @@ -38,7 +38,7 @@ public abstract class BinaryParser implements BlockReaderAdapter { /** Take a Info protocol buffer containing a date and convert it into a java Date object */ protected Date getDate(Osmformat.Info info) { if (info.hasTimestamp()) { - return new Date(date_granularity * (long) info.getTimestamp()); + return new Date(date_granularity * info.getTimestamp()); } else return NODATE; } diff --git a/src.java/crosby/binary/BinarySerializer.java b/src.java/crosby/binary/BinarySerializer.java index 683c2b1..6ec1e56 100644 --- a/src.java/crosby/binary/BinarySerializer.java +++ b/src.java/crosby/binary/BinarySerializer.java @@ -82,8 +82,8 @@ public class BinarySerializer implements Closeable, Flushable { /** How many primitives have been seen in this batch */ protected int batch_size = 0; protected int total_entities = 0; - private StringTable stringtable = new StringTable(); - protected List groups = new ArrayList(); + private final StringTable stringtable = new StringTable(); + protected List groups = new ArrayList<>(); protected BlockOutputStream output; public BinarySerializer(BlockOutputStream output) { diff --git a/src.java/crosby/binary/StringTable.java b/src.java/crosby/binary/StringTable.java index b7785e2..e2c1d2d 100644 --- a/src.java/crosby/binary/StringTable.java +++ b/src.java/crosby/binary/StringTable.java @@ -37,11 +37,7 @@ public class StringTable { private String set[]; public void incr(String s) { - if (counts.containsKey(s)) { - counts.put(s, new Integer(counts.get(s).intValue() + 1)); - } else { - counts.put(s, new Integer(1)); - } + counts.merge(s, 1, Integer::sum); } /** After the stringtable has been built, return the offset of a string in it. @@ -51,17 +47,11 @@ public class StringTable { * @return */ public int getIndex(String s) { - return stringmap.get(s).intValue(); + return stringmap.get(s); } public void finish() { - Comparator comparator = new Comparator() { - @Override - public int compare(final String s1, String s2) { - int diff = counts.get(s2) - counts.get(s1); - return diff; - } - }; + Comparator comparator = (s1, s2) -> counts.get(s2) - counts.get(s1); /* Sort the stringtable */ @@ -114,15 +104,15 @@ public class StringTable { Arrays.sort(set, Math.min(1 << 14, set.length-1), Math.min(1 << 21, set.length-1), comparator); } - stringmap = new HashMap(2 * set.length); + stringmap = new HashMap<>(2 * set.length); for (int i = 0; i < set.length; i++) { - stringmap.put(set[i], new Integer(i+1)); // Index 0 is reserved for use as a delimiter. + stringmap.put(set[i], i + 1); // Index 0 is reserved for use as a delimiter. } counts = null; } public void clear() { - counts = new HashMap(100); + counts = new HashMap<>(100); stringmap = null; set = null; } @@ -131,8 +121,9 @@ public class StringTable { Osmformat.StringTable.Builder builder = Osmformat.StringTable .newBuilder(); builder.addS(ByteString.copyFromUtf8("")); // Add a unused string at offset 0 which is used as a delimiter. - for (int i = 0; i < set.length; i++) - builder.addS(ByteString.copyFromUtf8(set[i])); + for (String s : set) { + builder.addS(ByteString.copyFromUtf8(s)); + } return builder; } } diff --git a/src.java/crosby/binary/file/BlockOutputStream.java b/src.java/crosby/binary/file/BlockOutputStream.java index 6932b5a..b0c6331 100644 --- a/src.java/crosby/binary/file/BlockOutputStream.java +++ b/src.java/crosby/binary/file/BlockOutputStream.java @@ -69,6 +69,6 @@ public class BlockOutputStream { } OutputStream outwrite; - List writtenblocks = new ArrayList(); + List writtenblocks = new ArrayList<>(); CompressFlags compression; } From 712c8cacf75be81b9d893914d11d48d36d277cb9 Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Sun, 3 Jan 2021 23:39:46 +0100 Subject: [PATCH 4/6] Use Java array syntax --- src.java/crosby/binary/BinaryParser.java | 2 +- src.java/crosby/binary/StringTable.java | 2 +- src.java/crosby/binary/file/FileBlock.java | 2 +- src.java/crosby/binary/file/FileBlockHead.java | 4 ++-- src.java/crosby/binary/file/FileBlockPosition.java | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src.java/crosby/binary/BinaryParser.java b/src.java/crosby/binary/BinaryParser.java index f32d41e..7a1d9c1 100644 --- a/src.java/crosby/binary/BinaryParser.java +++ b/src.java/crosby/binary/BinaryParser.java @@ -33,7 +33,7 @@ public abstract class BinaryParser implements BlockReaderAdapter { private long lat_offset; private long lon_offset; protected int date_granularity; - private String strings[]; + private String[] strings; /** Take a Info protocol buffer containing a date and convert it into a java Date object */ protected Date getDate(Osmformat.Info info) { diff --git a/src.java/crosby/binary/StringTable.java b/src.java/crosby/binary/StringTable.java index e2c1d2d..7f704b0 100644 --- a/src.java/crosby/binary/StringTable.java +++ b/src.java/crosby/binary/StringTable.java @@ -34,7 +34,7 @@ public class StringTable { private HashMap counts; private HashMap stringmap; - private String set[]; + private String[] set; public void incr(String s) { counts.merge(s, 1, Integer::sum); diff --git a/src.java/crosby/binary/file/FileBlock.java b/src.java/crosby/binary/file/FileBlock.java index 4b4e146..23eb94a 100644 --- a/src.java/crosby/binary/file/FileBlock.java +++ b/src.java/crosby/binary/file/FileBlock.java @@ -65,7 +65,7 @@ public class FileBlock extends FileBlockBase { Deflater deflater = new Deflater(); deflater.setInput(data.toByteArray()); deflater.finish(); - byte out[] = new byte[size]; + byte[] out = new byte[size]; deflater.deflate(out); if (!deflater.finished()) { diff --git a/src.java/crosby/binary/file/FileBlockHead.java b/src.java/crosby/binary/file/FileBlockHead.java index bf7259a..53113c6 100644 --- a/src.java/crosby/binary/file/FileBlockHead.java +++ b/src.java/crosby/binary/file/FileBlockHead.java @@ -50,7 +50,7 @@ public class FileBlockHead extends FileBlockReference { throw new FileFormatException("Unexpectedly long header "+MAX_HEADER_SIZE+ " bytes. Possibly corrupt file."); } - byte buf[] = new byte[headersize]; + byte[] buf = new byte[headersize]; datinput.readFully(buf); // System.out.format("Read buffer for header of %d bytes\n",buf.length); Fileformat.BlobHeader header = Fileformat.BlobHeader @@ -90,7 +90,7 @@ public class FileBlockHead extends FileBlockReference { */ FileBlock readContents(InputStream input) throws IOException { DataInputStream datinput = new DataInputStream(input); - byte buf[] = new byte[getDatasize()]; + byte[] buf = new byte[getDatasize()]; datinput.readFully(buf); return parseData(buf); } diff --git a/src.java/crosby/binary/file/FileBlockPosition.java b/src.java/crosby/binary/file/FileBlockPosition.java index 044de55..5f1d3c4 100644 --- a/src.java/crosby/binary/file/FileBlockPosition.java +++ b/src.java/crosby/binary/file/FileBlockPosition.java @@ -43,13 +43,13 @@ public class FileBlockPosition extends FileBlockBase { } /** Parse out and decompress the data part of a fileblock helper function. */ - FileBlock parseData(byte buf[]) throws InvalidProtocolBufferException { + FileBlock parseData(byte[] buf) throws InvalidProtocolBufferException { FileBlock out = FileBlock.newInstance(type, null, indexdata); Fileformat.Blob blob = Fileformat.Blob.parseFrom(buf); if (blob.hasRaw()) { out.data = blob.getRaw(); } else if (blob.hasZlibData()) { - byte buf2[] = new byte[blob.getRawSize()]; + byte[] buf2 = new byte[blob.getRawSize()]; Inflater decompresser = new Inflater(); decompresser.setInput(blob.getZlibData().toByteArray()); // decompresser.getRemaining(); @@ -85,7 +85,7 @@ public class FileBlockPosition extends FileBlockBase { public FileBlock read(InputStream input) throws IOException { if (input instanceof FileInputStream) { ((FileInputStream) input).getChannel().position(data_offset); - byte buf[] = new byte[getDatasize()]; + byte[] buf = new byte[getDatasize()]; (new DataInputStream(input)).readFully(buf); return parseData(buf); } else { From 9985bf46ad72f1ce3f984343dcf3ef22a5106485 Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Sun, 3 Jan 2021 23:40:40 +0100 Subject: [PATCH 5/6] Update Javadoc --- src.java/crosby/binary/BinaryParser.java | 6 ++---- src.java/crosby/binary/BinarySerializer.java | 4 ++-- src.java/crosby/binary/StringTable.java | 14 +++++++++----- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src.java/crosby/binary/BinaryParser.java b/src.java/crosby/binary/BinaryParser.java index 7a1d9c1..16305bb 100644 --- a/src.java/crosby/binary/BinaryParser.java +++ b/src.java/crosby/binary/BinaryParser.java @@ -23,7 +23,6 @@ import java.util.List; import com.google.protobuf.InvalidProtocolBufferException; -import crosby.binary.Osmformat; import crosby.binary.file.BlockReaderAdapter; import crosby.binary.file.FileBlock; import crosby.binary.file.FileBlockPosition; @@ -47,8 +46,8 @@ public abstract class BinaryParser implements BlockReaderAdapter { /** Get a string based on the index used. * * Index 0 is reserved to use as a delimiter, therefore, index 1 corresponds to the first string in the table - * @param id - * @return + * @param id the index + * @return the string at the given index */ protected String getStringById(int id) { return strings[id]; @@ -56,7 +55,6 @@ public abstract class BinaryParser implements BlockReaderAdapter { @Override public void handleBlock(FileBlock message) { - // TODO Auto-generated method stub try { if (message.getType().equals("OSMHeader")) { Osmformat.HeaderBlock headerblock = Osmformat.HeaderBlock diff --git a/src.java/crosby/binary/BinarySerializer.java b/src.java/crosby/binary/BinarySerializer.java index 6ec1e56..999118e 100644 --- a/src.java/crosby/binary/BinarySerializer.java +++ b/src.java/crosby/binary/BinarySerializer.java @@ -69,11 +69,11 @@ public class BinarySerializer implements Closeable, Flushable { this.batch_limit = batch_limit; } - // Paramaters affecting the output size. + // Parameters affecting the output size. protected final int MIN_DENSE = 10; protected int batch_limit = 4000; - // Parmaters affecting the output. + // Parameters affecting the output. protected int granularity = 100; protected int date_granularity = 1000; diff --git a/src.java/crosby/binary/StringTable.java b/src.java/crosby/binary/StringTable.java index 7f704b0..657c7d9 100644 --- a/src.java/crosby/binary/StringTable.java +++ b/src.java/crosby/binary/StringTable.java @@ -24,7 +24,7 @@ import java.util.HashMap; import com.google.protobuf.ByteString; /** - * Class for mapping a set of strings to integers, giving frequently occuring + * Class for mapping a set of strings to integers, giving frequently occurring * strings small integers. */ public class StringTable { @@ -36,6 +36,10 @@ public class StringTable { private HashMap stringmap; private String[] set; + /** + * Increments the count of the given string + * @param s the string + */ public void incr(String s) { counts.merge(s, 1, Integer::sum); } @@ -43,8 +47,8 @@ public class StringTable { /** After the stringtable has been built, return the offset of a string in it. * * Note, value '0' is reserved for use as a delimiter and will not be returned. - * @param s - * @return + * @param s the string to lookup + * @return the offset of the string */ public int getIndex(String s) { return stringmap.get(s); @@ -78,7 +82,7 @@ public class StringTable { So, when I decide on the master stringtable to use, I put the 127 most frequently occurring strings into A (accomplishing goal 1), and sort them by frequency (to accomplish goal 2), but for B and C, which contain the less progressively less frequently encountered strings, I sort - them lexiconographically, to maximize goal 3 and ignoring goal 2. + them lexicographically, to maximize goal 3 and ignoring goal 2. Goal 1 is the most important. Goal 2 helped enough to be worth it, and goal 3 was pretty minor, but all should be re-benchmarked. @@ -93,7 +97,7 @@ public class StringTable { // Sort based on the frequency. Arrays.sort(set, comparator); // Each group of keys that serializes to the same number of bytes is - // sorted lexiconographically. + // sorted lexicographically. // to maximize deflate compression. // Don't sort the first array. There's not likely to be much benefit, and we want frequent values to be small. From d5bdd020c1caf1972d28889ec687f16043a94c4e Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Sun, 3 Jan 2021 23:50:43 +0100 Subject: [PATCH 6/6] Replace Error with adequate Exceptions Sometimes wrapped in UncheckedIOException to not change the method signatures. --- src.java/crosby/binary/BinaryParser.java | 6 +++--- src.java/crosby/binary/BinarySerializer.java | 5 ++--- src.java/crosby/binary/file/BlockOutputStream.java | 2 +- src.java/crosby/binary/file/FileBlock.java | 10 ++++++---- src.java/crosby/binary/file/FileBlockPosition.java | 10 +++++----- src.java/crosby/binary/file/FileFormatException.java | 4 ++++ 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src.java/crosby/binary/BinaryParser.java b/src.java/crosby/binary/BinaryParser.java index 16305bb..7880ce0 100644 --- a/src.java/crosby/binary/BinaryParser.java +++ b/src.java/crosby/binary/BinaryParser.java @@ -18,6 +18,7 @@ package crosby.binary; +import java.io.UncheckedIOException; import java.util.Date; import java.util.List; @@ -26,6 +27,7 @@ import com.google.protobuf.InvalidProtocolBufferException; import crosby.binary.file.BlockReaderAdapter; import crosby.binary.file.FileBlock; import crosby.binary.file.FileBlockPosition; +import crosby.binary.file.FileFormatException; public abstract class BinaryParser implements BlockReaderAdapter { protected int granularity; @@ -66,9 +68,7 @@ public abstract class BinaryParser implements BlockReaderAdapter { parse(primblock); } } catch (InvalidProtocolBufferException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - throw new Error("ParseError"); // TODO + throw new UncheckedIOException(new FileFormatException(e)); } } diff --git a/src.java/crosby/binary/BinarySerializer.java b/src.java/crosby/binary/BinarySerializer.java index 999118e..456c219 100644 --- a/src.java/crosby/binary/BinarySerializer.java +++ b/src.java/crosby/binary/BinarySerializer.java @@ -20,6 +20,7 @@ package crosby.binary; import java.io.Closeable; import java.io.Flushable; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.List; @@ -143,9 +144,7 @@ public class BinarySerializer implements Closeable, Flushable { output.write(FileBlock.newInstance("OSMData", message .toByteString(), null)); } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - throw new Error(e); + throw new UncheckedIOException(e); } finally { batch_size = 0; groups.clear(); diff --git a/src.java/crosby/binary/file/BlockOutputStream.java b/src.java/crosby/binary/file/BlockOutputStream.java index b0c6331..dc65fe6 100644 --- a/src.java/crosby/binary/file/BlockOutputStream.java +++ b/src.java/crosby/binary/file/BlockOutputStream.java @@ -44,7 +44,7 @@ public class BlockOutputStream { else if (s.equals("deflate")) compression = CompressFlags.DEFLATE; else - throw new Error("Unknown compression type: " + s); + throw new IllegalArgumentException("Unknown compression type: " + s); } /** Write a block with the stream's default compression flag */ diff --git a/src.java/crosby/binary/file/FileBlock.java b/src.java/crosby/binary/file/FileBlock.java index 23eb94a..16c1ce3 100644 --- a/src.java/crosby/binary/file/FileBlock.java +++ b/src.java/crosby/binary/file/FileBlock.java @@ -18,10 +18,12 @@ package crosby.binary.file; import java.io.DataOutputStream; +import java.io.EOFException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.UncheckedIOException; import java.util.Arrays; import java.util.zip.Deflater; @@ -48,13 +50,13 @@ public class FileBlock extends FileBlockBase { if (blob != null && blob.size() > MAX_BODY_SIZE/2) { System.err.println("Warning: Fileblock has body size too large and may be considered corrupt"); if (blob != null && blob.size() > MAX_BODY_SIZE-1024*1024) { - throw new Error("This file has too many entities in a block. Parsers will reject it."); + throw new IllegalArgumentException("This file has too many entities in a block. Parsers will reject it."); } } if (indexdata != null && indexdata.size() > MAX_HEADER_SIZE/2) { System.err.println("Warning: Fileblock has indexdata too large and may be considered corrupt"); if (indexdata != null && indexdata.size() > MAX_HEADER_SIZE-512) { - throw new Error("This file header is too large. Parsers will reject it."); + throw new IllegalArgumentException("This file header is too large. Parsers will reject it."); } } return new FileBlock(type, blob, indexdata); @@ -77,7 +79,7 @@ public class FileBlock extends FileBlockBase { deflater.deflate(out, deflater.getTotalOut(), out.length - deflater.getTotalOut()); if (!deflater.finished()) { - throw new Error("Internal error in compressor"); + throw new UncheckedIOException(new EOFException()); } } ByteString compressed = ByteString.copyFrom(out, 0, deflater @@ -103,7 +105,7 @@ public class FileBlock extends FileBlockBase { if (flags == CompressFlags.DEFLATE) deflateInto(blobbuilder); else - throw new Error("Compression flag not understood"); + throw new IllegalArgumentException("Compression flag not understood"); } Fileformat.Blob blob = blobbuilder.build(); diff --git a/src.java/crosby/binary/file/FileBlockPosition.java b/src.java/crosby/binary/file/FileBlockPosition.java index 5f1d3c4..c463ecf 100644 --- a/src.java/crosby/binary/file/FileBlockPosition.java +++ b/src.java/crosby/binary/file/FileBlockPosition.java @@ -21,6 +21,7 @@ import java.io.DataInputStream; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.util.zip.DataFormatException; import java.util.zip.Inflater; @@ -56,8 +57,7 @@ public class FileBlockPosition extends FileBlockBase { try { decompresser.inflate(buf2); } catch (DataFormatException e) { - e.printStackTrace(); - throw new Error(e); + throw new UncheckedIOException(new FileFormatException(e)); } assert (decompresser.finished()); decompresser.end(); @@ -89,7 +89,7 @@ public class FileBlockPosition extends FileBlockBase { (new DataInputStream(input)).readFully(buf); return parseData(buf); } else { - throw new Error("Random access binary reads require seekability"); + throw new IllegalArgumentException("Random access binary reads require seekability"); } } @@ -98,12 +98,12 @@ public class FileBlockPosition extends FileBlockBase { * stored. */ public ByteString serialize() { - throw new Error("TODO"); + throw new UnsupportedOperationException("TODO"); } /** TODO: Parse a serialized representation of this block reference */ static FileBlockPosition parseFrom(ByteString b) { - throw new Error("TODO"); + throw new UnsupportedOperationException("TODO"); } protected int datasize; diff --git a/src.java/crosby/binary/file/FileFormatException.java b/src.java/crosby/binary/file/FileFormatException.java index 8a10d88..27ec8a5 100644 --- a/src.java/crosby/binary/file/FileFormatException.java +++ b/src.java/crosby/binary/file/FileFormatException.java @@ -25,6 +25,10 @@ public class FileFormatException extends IOException { super(string); } + public FileFormatException(Throwable cause) { + super(cause); + } + /** * */