On Mon, 10 Aug 2026 17:36:35 GMT, Naoto Sato <[email protected]> wrote:
> This PR implements [JEP 540: Simple JSON API > (Incubator)](https://openjdk.org/jeps/540). > > It adds the `jdk.incubator.json` module which provides APIs for reading and > writing JSON documents as specified by [RFC > 8259](https://datatracker.ietf.org/doc/html/rfc8259). This is an incubating > API. > > API documentation: > https://cr.openjdk.org/~naoto/json/javadoc/api/jdk.incubator.json/module-summary.html > Co-authored-by: Justin Lu > ([@justin-curtis-lu](https://github.com/justin-curtis-lu)) > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Read through the main body code a bit. Did not read through spec/comments or tests. Currently, JSON elements can be like legacy substring that hold references to the backing contents, which can be costly if the elements are passed around to construct new JSON trees. Is the recommended way to do `toString` and parse again? src/jdk.incubator.json/share/classes/jdk/incubator/json/Json.java line 130: > 128: case JsonObject jo -> toDisplayString(jo, s, col, indent, > isField); > 129: case JsonArray ja -> toDisplayString(ja, s, col, indent, > isField); > 130: default -> s.append(" ".repeat(isField ? 1 : > col)).append(jv); Suggestion: default -> s.repeat(' ', isField ? 1 : col).append(jv); src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonArray.java line 78: > 76: .stream() > 77: .map(Objects::requireNonNull) > 78: .collect(Collectors.toCollection(ArrayList::new)) Suggestion: .collect(Collectors.toList()) Or you can use `src.stream().map(Objects::requireNonNull).toList()` - this list does not NPE upon `contains(null)` but is immutable. src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonBooleanImpl.java line 35: > 33: public final class JsonBooleanImpl implements JsonBoolean, JsonValueImpl { > 34: > 35: private final Boolean theBoolean; No need to use java.lang.Boolean, boolean is sufficient as far as I see in the parser. src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonNumberImpl.java line 58: > 56: public int asInt() { > 57: return numInteger.get().orElseThrow(() -> > 58: Utils.composeError(this, this + " cannot be represented as an > int.")); `orElseThrow` unforunately requires a capturing lambda - ugliness but C2 should be able to deal with these. Or the language need some optimizations around this-capturing lambdas. src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonParser.java line 117: > 115: // Check for empty case > 116: if (charEquals('}')) { > 117: return new JsonObjectImpl(Map.of(), startO, doc); `Map.of` NPEs on `containsKey` and `containsValue`, not sure if you desire this behavior given the unmodifiable-wrapped linked hash map doesn't have this behavior. src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonParser.java line 222: > 220: // Check for empty case > 221: if (charEquals(']')) { > 222: return new JsonArrayImpl(List.of(), startO, doc); Similar observation for `List.of()` versus unmodifiable-wrapped `ArrayList` as for `Map.of()` and unmodifiable-wrapped `LinkedHashMap`. ------------- PR Review: https://git.openjdk.org/jdk/pull/32282#pullrequestreview-4900832228 PR Comment: https://git.openjdk.org/jdk/pull/32282#issuecomment-5247922809 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3753248891 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3753267357 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3755011610 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3755017757 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3755038709 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3755044691
