Enhance performance of AbstractPatternJsonProvider #688
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- PatternLayoutBase creates a new StringBuilder for every event it must serialize causing garbage and unnecessary pressure on the GC. Try to reuse a StringBuilder per thread. - Filtering of empty fields is getting values multiple times from the event which becomes expensive when the value is computed by a PatternLayout. Use Jackson's FilteringJsonGenerator instead. - Make use of the most appropriate JsonGenerator#write() method instead of using writeObject() for "simple" value types. Using writeObject() delegates to the underlying ObjectCode which is slower and produces lot more garbage. - Refactor the code to make it easier to read and understand.
philsttr
reviewed
Oct 31, 2021
src/main/java/net/logstash/logback/composite/JsonWritingUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/net/logstash/logback/pattern/AbstractJsonPatternParser.java
Show resolved
Hide resolved
src/main/java/net/logstash/logback/pattern/AbstractJsonPatternParser.java
Outdated
Show resolved
Hide resolved
src/main/java/net/logstash/logback/pattern/AbstractJsonPatternParser.java
Outdated
Show resolved
Hide resolved
They seem to have good coverage already... Do you have additional scenarios in mind? |
No. I haven't looked at them. I just wanted to confirm. I trust your judgement. |
DelegatingNodeWriter is private and used only by OmitEmptyFieldWriter. Suppress the class and move the delegate field directly into the sub-class.
…s instead of logging an ERROR status AbstractJsonPatternParser now throws a JsonPatternException in case of invalid pattern. It is up to the AbstractPatternLayoutJsonProvider to catch the exception, log an error status and decide how it should behave. Invalid PatternLayout formats are now detected and throw a JsonPatternException as well. Detection is done by configuring the PatternLayout instances with a custom Logback context that throws an exception when an ERROR status is logged. The former implementation used to ignore fields with an invalid pattern layout while keeping the other. This results in a partial JSON output with only the valid fields. This behaviour is now changed and the provider reverts to producing nothing in case of bad configuration. These changes address issue #686.
… patterns instead of logging an ERROR status" This reverts commit 414f482. This changes will be committed in a separate branch.
|
I'm done with this PR. |
philsttr
approved these changes
Nov 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses multiple performance issues of the AbstractJsonPatternParser.
JsonGenerator#write()method instead of usingwriteObject()for "simple" value types. UsingwriteObject()delegates to the underlying ObjectCodec which is slower and produces lot more garbage.Benchmarks show that these changes make the AbstractPatternJsonProvider 4 times faster than LLE 6.6 and more importantly, it does not produce garbage anymore.