Skip to content

Use garbage-free formatter for s and S patterns #3338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Dec 31, 2024
Merged

Use garbage-free formatter for s and S patterns #3338

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cba3b5f
Use garbage-free formatter for `s` and `S` patterns
ppkarwasz Dec 27, 2024
69fa9cc
Fix failing test
ppkarwasz Dec 28, 2024
bfce29a
Apply suggestions from code review
ppkarwasz Dec 28, 2024
7879316
Apply review suggestions(2)
ppkarwasz Dec 28, 2024
0779964
Improve Javadoc
ppkarwasz Dec 28, 2024
1483158
Miscellaneous fixes
ppkarwasz Dec 28, 2024
acaaa69
Make `AbstractFormatter` private
ppkarwasz Dec 28, 2024
60e9002
Remove Java 17 method calls
ppkarwasz Dec 28, 2024
a44e3c9
Final corrections
ppkarwasz Dec 28, 2024
0e8ddb3
Move `escapeLiteral` to `PatternSequence`
ppkarwasz Dec 28, 2024
2279c11
Fix precision detection
ppkarwasz Dec 30, 2024
756ee61
Improve Javadoc of `tryMerge`
ppkarwasz Dec 30, 2024
412baee
Make benchmark JAR executable again
ppkarwasz Dec 30, 2024
cb1a159
Document GC characteristics of instant formatters
ppkarwasz Dec 30, 2024
0b198fc
Improve performance
ppkarwasz Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 34 additions & 66 deletions ...g/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.apache.logging.log4j.core.util.internal.instant;

import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.sequencePattern;
import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -32,10 +31,9 @@
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.apache.logging.log4j.core.time.MutableInstant;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.CompositePatternSequence;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.DynamicPatternSequence;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.PatternSequence;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.StaticPatternSequence;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.DateTimeFormatterPatternFormatterFactory;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.PatternFormatterFactory;
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.SecondPatternFormatterFactory;
import org.apache.logging.log4j.util.Constants;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand All @@ -47,112 +45,81 @@ class InstantPatternDynamicFormatterTest {
@ParameterizedTest
@MethodSource("sequencingTestCases")
void sequencing_should_work(
final String pattern, final ChronoUnit thresholdPrecision, final List<PatternSequence> expectedSequences) {
final List<PatternSequence> actualSequences = sequencePattern(pattern, thresholdPrecision);
final String pattern,
final ChronoUnit thresholdPrecision,
final List<PatternFormatterFactory> expectedSequences) {
final List<PatternFormatterFactory> actualSequences = sequencePattern(pattern, thresholdPrecision);
assertThat(actualSequences).isEqualTo(expectedSequences);
}

static List<Arguments> sequencingTestCases() {
final List<Arguments> testCases = new ArrayList<>();

// `SSSX` should be treated constant for daily updates
testCases.add(Arguments.of("SSSX", ChronoUnit.DAYS, singletonList(pCom(pDyn("SSS"), pDyn("X")))));
testCases.add(Arguments.of("SSSX", ChronoUnit.DAYS, asList(pMilliSec(), pDyn("X"))));

// `yyyyMMddHHmmssSSSX` instant cache updated hourly
testCases.add(Arguments.of(
"yyyyMMddHHmmssSSSX",
ChronoUnit.HOURS,
asList(
pCom(pDyn("yyyy"), pDyn("MM"), pDyn("dd"), pDyn("HH")),
pCom(pDyn("mm"), pDyn("ss"), pDyn("SSS")),
pDyn("X"))));
asList(pDyn("yyyyMMddHH", ChronoUnit.HOURS), pDyn("mm"), pSec("", 3), pDyn("X"))));

// `yyyyMMddHHmmssSSSX` instant cache updated per minute
testCases.add(Arguments.of(
"yyyyMMddHHmmssSSSX",
ChronoUnit.MINUTES,
asList(
pCom(pDyn("yyyy"), pDyn("MM"), pDyn("dd"), pDyn("HH"), pDyn("mm")),
pCom(pDyn("ss"), pDyn("SSS")),
pDyn("X"))));
asList(pDyn("yyyyMMddHHmm", ChronoUnit.MINUTES), pSec("", 3), pDyn("X"))));

// ISO9601 instant cache updated daily
final String iso8601InstantPattern = "yyyy-MM-dd'T'HH:mm:ss.SSSX";
testCases.add(Arguments.of(
iso8601InstantPattern,
ChronoUnit.DAYS,
asList(
pCom(pDyn("yyyy"), pSta("-"), pDyn("MM"), pSta("-"), pDyn("dd"), pSta("T")),
pCom(
pDyn("HH"),
pSta(":"),
pDyn("mm"),
pSta(":"),
pDyn("ss"),
pSta("."),
pDyn("SSS"),
pDyn("X")))));
pDyn("yyyy'-'MM'-'dd'T'", ChronoUnit.DAYS),
pDyn("HH':'mm':'", ChronoUnit.MINUTES),
pSec(".", 3),
pDyn("X"))));

// ISO9601 instant cache updated per minute
testCases.add(Arguments.of(
iso8601InstantPattern,
ChronoUnit.MINUTES,
asList(
pCom(
pDyn("yyyy"),
pSta("-"),
pDyn("MM"),
pSta("-"),
pDyn("dd"),
pSta("T"),
pDyn("HH"),
pSta(":"),
pDyn("mm"),
pSta(":")),
pCom(pDyn("ss"), pSta("."), pDyn("SSS")),
pDyn("X"))));
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));

// ISO9601 instant cache updated per second
testCases.add(Arguments.of(
iso8601InstantPattern,
ChronoUnit.SECONDS,
asList(
pCom(
pDyn("yyyy"),
pSta("-"),
pDyn("MM"),
pSta("-"),
pDyn("dd"),
pSta("T"),
pDyn("HH"),
pSta(":"),
pDyn("mm"),
pSta(":"),
pDyn("ss"),
pSta(".")),
pDyn("SSS"),
pDyn("X"))));
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));

// Seconds and micros
testCases.add(Arguments.of(
"HH:mm:ss.SSSSSS", ChronoUnit.MINUTES, asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 6))));

return testCases;
}

private static CompositePatternSequence pCom(final PatternSequence... sequences) {
return new CompositePatternSequence(asList(sequences));
private static DateTimeFormatterPatternFormatterFactory pDyn(final String pattern) {
return new DateTimeFormatterPatternFormatterFactory(pattern);
}

private static DateTimeFormatterPatternFormatterFactory pDyn(final String pattern, final ChronoUnit precision) {
return new DateTimeFormatterPatternFormatterFactory(pattern, precision);
}

private static DynamicPatternSequence pDyn(final String pattern) {
return new DynamicPatternSequence(pattern);
private static SecondPatternFormatterFactory pSec(String separator, int fractionalDigits) {
return new SecondPatternFormatterFactory(true, separator, fractionalDigits);
}

private static StaticPatternSequence pSta(final String literal) {
return new StaticPatternSequence(literal);
private static SecondPatternFormatterFactory pMilliSec() {
return new SecondPatternFormatterFactory(false, "", 3);
}

@ParameterizedTest
@ValueSource(
strings = {
// Basics
"S",
"SSSSSSS",
"SSSSSSSSS",
"n",
Expand All @@ -163,8 +130,7 @@ private static StaticPatternSequence pSta(final String literal) {
"yyyy-MM-dd HH:mm:ss,SSSSSSS",
"yyyy-MM-dd HH:mm:ss,SSSSSSSS",
"yyyy-MM-dd HH:mm:ss,SSSSSSSSS",
"yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS",
"yyyy-MM-dd'T'HH:mm:ss.SXXX"
"yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS"
})
void should_recognize_patterns_of_nano_precision(final String pattern) {
assertPatternPrecision(pattern, ChronoUnit.NANOS);
Expand Down Expand Up @@ -351,7 +317,9 @@ static Stream<Arguments> formatterInputs(final String pattern) {

private static MutableInstant randomInstant() {
final MutableInstant instant = new MutableInstant();
final long epochSecond = RANDOM.nextInt(1_621_280_470); // 2021-05-17 21:41:10
// In the 1970's some time zones had sub-minute offsets to UTC, e.g., Africa/Monrovia
final int startEighties = 315_532_800;
final long epochSecond = startEighties + RANDOM.nextInt(1_621_280_470 - startEighties); // 2021-05-17 21:41:10
final int epochSecondNano = randomNanos();
instant.initFromEpochSecond(epochSecond, epochSecondNano);
return instant;
Expand Down
Loading
Loading