[
https://issues.apache.org/jira/browse/LOG4J2-1385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15315993#comment-15315993
]
Gary Gregory commented on LOG4J2-1385:
--------------------------------------
Yes, Apache Commons CSV 1.4 is now required.
CSV layouts _should_ be close to GC-free unless
{{org.apache.commons.csv.CSVFormat.getTrim()}} is true (it's false by default
and Log4j does not set it IIRC). I need to work on that one for the next CSV
release.
The GC profile should be _a lot_ better with Commons CSV 1.4 and Log4j 2.6.1
but it might not be 100% GC free because a key method in Commons CSV is
{{org.apache.commons.csv.CSVFormat.print(Object, Appendable, boolean)}}:
{code:java}
/**
* Prints the {@code value} as the next value on the line to {@code out}.
The value will be escaped or encapsulated
* as needed. Useful when one wants to avoid creating CSVPrinters.
*
* @param value
* value to output.
* @param out
* where to print the value
* @param newRecord
* if this a new record
* @throws IOException
* If an I/O error occurs
* @since 1.4
*/
public void print(final Object value, final Appendable out, final boolean
newRecord) throws IOException {
// null values are considered empty
// Only call CharSequence.toString() if you have to, helps GC-free use
cases.
CharSequence charSequence;
if (value == null) {
charSequence = nullString == null ? Constants.EMPTY : nullString;
} else {
charSequence = value instanceof CharSequence ? (CharSequence) value
: value.toString();
}
charSequence = getTrim() ? trim(charSequence) : charSequence;
this.print(value, charSequence, 0, charSequence.length(), out,
newRecord);
}
{code}
There is a {{trim()}} and a {{toString()}} in there that are not GC-free. I
think Log4j would hit those code paths for Objects like Level and numbers.
I just know it's a lot better now.
Since Commons CSV does not know about things like
{{org.apache.logging.log4j.util.StringBuilderFormattable}} it cannot take use
it but I do not think it would any way.
Looking at
{{org.apache.logging.log4j.core.layout.CsvLogEventLayout.toSerializable(LogEvent)}}:
{code:java}
@Override
public String toSerializable(final LogEvent event) {
final StringBuilder buffer = getStringBuilder();
// Revisit when 1.3 is out so that we do not need to create a new
// printer for each event.
final CSVFormat format = getFormat();
try {
format.print(event.getNanoTime(), buffer, true);
format.print(event.getTimeMillis(), buffer, false);
format.print(event.getLevel(), buffer, false);
format.print(event.getThreadId(), buffer, false);
format.print(event.getThreadName(), buffer, false);
format.print(event.getThreadPriority(), buffer, false);
format.print(event.getMessage().getFormattedMessage(), buffer,
false);
format.print(event.getLoggerFqcn(), buffer, false);
format.print(event.getLoggerName(), buffer, false);
format.print(event.getMarker(), buffer, false);
format.print(event.getThrownProxy(), buffer, false);
format.print(event.getSource(), buffer, false);
format.print(event.getContextMap(), buffer, false);
format.print(event.getContextStack(), buffer, false);
format.println(buffer);
return buffer.toString();
} catch (final IOException e) {
StatusLogger.getLogger().error(event.toString(), e);
return format.getCommentMarker() + " " + e;
}
}
{code}
This will create garbage through boxing :-( Perhaps Commons CSV should provide
primitive versions of {{org.apache.commons.csv.CSVFormat.print(Object,
Appendable, boolean)}} like
- {{org.apache.commons.csv.CSVFormat.print(int, Appendable, boolean)}}
- {{org.apache.commons.csv.CSVFormat.print(long, Appendable, boolean)}}
- ...
It looks like Log4j would only need the {{long}} version.
Thoughts?
> (GC) CSV layouts should not create a new CSVPrinter for each log event
> ----------------------------------------------------------------------
>
> Key: LOG4J2-1385
> URL: https://issues.apache.org/jira/browse/LOG4J2-1385
> Project: Log4j 2
> Issue Type: Improvement
> Components: Layouts
> Affects Versions: 2.5
> Reporter: Gary Gregory
> Assignee: Gary Gregory
> Fix For: 2.6.1
>
> Attachments: logging-log4j2.patch
>
>
> CSV layouts should not create a new CSVPrinter for each log event. This will
> likely require a new version of Apache Commons CVS, see [CSV-182].
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
