1
\$\begingroup\$

I'm looking to parse a reduced subset of the Valve Data Format (VDF). It is similar to JSON, allowing representation of key-value collections (maps) with arbitrary recursion. As a small example:

"root"
{
    "key1"      "value1"
    "key2"      "value2"
    "key3"
    {
        ...
    }
}

I'm taking on only a subset of the language (no comments; all keys and values enclosed in quotation marks; no escape sequences). Below is the code I've written to tokenize and iterate over input of the above form:

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.io.Reader;
import java.io.UncheckedIOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Iterator;

class TokenIterator implements Iterator<String>, Iterable<String>, AutoCloseable
{
    private static final int EOF = -1;
    private static final int BUFFER_SZ = 4 * 1024 * 1024;
    private static final String LPAREN = "{";
    private static final String RPAREN = "}";

    private final Reader reader;
    private final StringBuilder sb;
    private int ch;

    private TokenIterator(Reader reader, StringBuilder sb, int ch)
    {
        this.reader = reader;
        this.sb = sb;
        this.ch = ch;
    }

    static TokenIterator forPath(Path path)
    {
        try
        {
            Reader reader = new BufferedReader(new FileReader(path.toFile()), BUFFER_SZ);
            StringBuilder sb = new StringBuilder();
            return new TokenIterator(reader, sb, reader.read());
        }
        catch (IOException e)
        {
            throw new UncheckedIOException(e);
        }
    }

    @Override
    public boolean hasNext()
    {
        try 
        {
            while ((ch == '\n' || ch == '\r' || ch == '\t' || ch == ' '))
            {
                ch = reader.read();
            }
        }
        catch (IOException e) 
        {
            throw new UncheckedIOException(e);
        }
        return ch != EOF;
    }

    @Override
    public String next()
    {
        try
        {
            String result;
            switch (ch)
            {
                case '"':
                    done:
                    do
                    {
                        ch = reader.read();
                        switch (ch) {
                            case EOF:
                                throw new IllegalArgumentException("Reached EOF while reading quoted string");
                            case '"':
                                ch = reader.read();
                                result = sb.toString();
                                sb.setLength(0);
                                break done;
                            default:
                                sb.append(ch);
                        }
                    }
                    while (true);
                    break;
                case '{':
                    result = LPAREN;
                    break;
                case '}':
                    result = RPAREN;
                    break;
                default:
                    String message = String.format("Unexpected char '%x' at beginning of token", ch);
                    throw new IllegalArgumentException(message);
            }
            ch = reader.read();
            return result;
        }
        catch (IOException e) 
        {
            throw new UncheckedIOException(e);
        }
    }

    @Override
    public void close() throws Exception
    {
        reader.close();
    }

    @Override
    public Iterator<String> iterator()
    {
        return this;
    }

    public static void main(String[] args) {
        TokenIterator it = forPath(Paths.get("items_game.txt"));
        long start = System.currentTimeMillis();
        for (String s : it) {}
        System.out.printf("Token iteration time: %d", System.currentTimeMillis() - start);
    }

}

All constructive comments are welcome, but I'm most interested in improving performance. It's taking around 70ms to iterate through a 3.66MB, ~131k line file of this type on my machine (i7-7700HQ 2.8GHz processor, 16GB system memory), which I find disappointing.

\$\endgroup\$

1 Answer 1

0
\$\begingroup\$

You are using the older, legacy java.io.File and java.io.FileReader classes for opening a newer java.nio.file.Path object. The nio stands for "new IO", and can make use of high-efficiency byte-buffers and operating system calls for file reading and writing. You may (or may not) see an improvement if you use the newer Files.newBufferedReader(Path) method from java.nio.file to open the file.

(You cannot specify the buffer size using the newBufferedReader() method, but your 4MB buffer was likely not helping you since the operating system would limit itself to transferring the drive's block-size memory chunks anyway.)

The TokenIterator constructor is private, and only called from forPath(...). The forPath(...) method does preparatory work for the constructor, creating a BufferedReader, StringBuilder and reading one character from the reader to prime the TokenIterator. These are all internal details which could be hidden completely inside the constructor:

private final StringBuilder sb = new StringBuilder();

public TokenIterator(Path path) {
    try {
       reader = Files.newBufferedReader(path);
       ch = reader.read();
    } catch (IOException e) {
       throw new UncheckedIOException(e);
    }
}

TokenIterator is declared as an AutoCloseable resource, but you are not taking advantage of that in your main function, so possibly leaking a resource:

public static void main(String[] args) {

    try (TokenIterator it = new TokenIterator(Paths.get("item_games.txt"))) {
        long start = System.currentTimeMillis();

        for (String s : it) { /* no-op */ }

        System.out.printf("Token iteration time: %d", System.currentTimeMillis() - start);
    }  // TokenIterator is auto-closed here.

}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.