carterkozak commented on pull request #444: URL: https://github.com/apache/logging-log4j2/pull/444#issuecomment-735789644
> Even though I was the one who started using ParameterizedMessage.deepToString(), thinking about it again, I am not happy with the end result. This breaks the self-containment contract of the class. I would/might rather copy your (or all?) optimizations from ParameterizedMessage.deepToString() to somewhere in JsonWriter#writeString(). It's worth considering whether or not we're solving the same problem as ParameterizedMessage -- I don't think there's any reason to produce divergent implementations if the goal is to write the string value of an object to a StringBuilder. If the implementations need to be subtly different, than I agree it's safer to define them separately :-) > Indeed it smells awkward to have JSON quoting routines in multiple locations within the code base. I had shared my concerns about it in a mailing list thread. But in a nutshell, why do we even have JSON quoting routines anywhere in the code base except JsonWriter? JsonLayout already uses Jackson, hence it is covered. But for the rest... I think we should just keep them for backward compatibility and remove them in master. Yep, this is a draft PR and I don't think we would want to combine the two implementations, however I do think it's worth considering using a similar algorithm for more efficient encoding in JTL. The idea is instead of using a second StringBuilder for intermediate output, we can write directly onto the final stringbuilder, then efficiently escape data. The `escapeJson` function counts characters which need to be escaped, and scans backward from the escaped length moving characters into place in O(n) time without additional memory requirements. ---------------------------------------------------------------- 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: [email protected]
