rmannibucau commented on a change in pull request #40: JOHNZON-209 Fix
JsonObject#toString() to escape key names.
URL: https://github.com/apache/johnzon/pull/40#discussion_r277176951
##########
File path:
johnzon-core/src/main/java/org/apache/johnzon/core/JsonObjectImpl.java
##########
@@ -147,7 +147,7 @@ public String toString() {
while (hasNext) {
final Map.Entry<String, JsonValue> entry = it.next();
- builder.append('"').append(entry.getKey()).append("\":");
+
builder.append('"').append(Strings.escape(entry.getKey())).append("\":");
Review comment:
This relies on a global buffer cache, we likely want to use the builder -
object builder or parser factory - buffer factory and not the global one which
is there for compatibility IIRC.
It also slows down default case - not escaped - which is a concern because
toString is a serialization solution for jsonobject for jsonb and some other
cases.
From memory, caching was not efficient until you are lock free and hash free.
Alternatively we can make Strings with a method taking a builder and without
the buffer logic we could call if the key contains characters to convert in
unicode or escapable, in this last case the check should be fast and conversion
can surely be done without buffer since it is very unlikely it needs to be
done. Needs a small benchmark but it sounds the simplest and fastest to me
assuming escaping is rare (from my experience it is for keys). Also iterating
over the char[] instead of String with chartAt will be faster.
Wdyt? Can you try to make this escape call a bit less impacting?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
