Skip to content

Commit 1e36b3e

Browse files
committed
Reduce inlined code in log field functions
More inlined code -> bigger code size -> possibly worse performance, and also more internal API that must be exposed with `@PublishedApi`. So reducing inlined code is good.
1 parent 3784c0a commit 1e36b3e

File tree

5 files changed

+171
-115
lines changed

5 files changed

+171
-115
lines changed

api/devlog-kotlin.api

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ public abstract interface class dev/hermannm/devlog/HasLoggingContext {
1717
public final class dev/hermannm/devlog/LogBuilder {
1818
public static final fun addField-impl (Ldev/hermannm/devlog/LogEvent;Ldev/hermannm/devlog/LogField;)V
1919
public static final fun addFields-impl (Ldev/hermannm/devlog/LogEvent;Ljava/util/Collection;)V
20+
public static final fun addLogFieldOfType-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/Object;Lkotlin/reflect/KType;)V
21+
public static final fun addStringLogField-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/Object;)V
2022
public static final synthetic fun box-impl (Ldev/hermannm/devlog/LogEvent;)Ldev/hermannm/devlog/LogBuilder;
2123
public static fun constructor-impl (Ldev/hermannm/devlog/LogEvent;)Ldev/hermannm/devlog/LogEvent;
2224
public fun equals (Ljava/lang/Object;)Z
@@ -46,19 +48,14 @@ public final class dev/hermannm/devlog/LogEventJvm {
4648
}
4749

4850
public final class dev/hermannm/devlog/LogField {
49-
public fun <init> (Ljava/lang/String;Ljava/lang/String;Z)V
5051
public fun equals (Ljava/lang/Object;)Z
5152
public fun hashCode ()I
5253
public fun toString ()Ljava/lang/String;
5354
}
5455

55-
public final class dev/hermannm/devlog/LogFieldJvm {
56-
public static final fun fieldValueShouldUseToString (Ljava/lang/Object;)Z
57-
}
58-
5956
public final class dev/hermannm/devlog/LogFieldKt {
60-
public static final field JSON_NULL_VALUE Ljava/lang/String;
61-
public static final field LOG_FIELD_JSON_FORMAT Lkotlinx/serialization/json/Json;
57+
public static final fun createLogFieldOfType (Ljava/lang/String;Ljava/lang/Object;Lkotlin/reflect/KType;)Ldev/hermannm/devlog/LogField;
58+
public static final fun createStringLogField (Ljava/lang/String;Ljava/lang/Object;)Ldev/hermannm/devlog/LogField;
6259
public static final fun field (Ljava/lang/String;Ljava/lang/Object;Lkotlinx/serialization/SerializationStrategy;)Ldev/hermannm/devlog/LogField;
6360
public static final fun rawJson (Ljava/lang/String;Z)Lkotlinx/serialization/json/JsonElement;
6461
public static synthetic fun rawJson$default (Ljava/lang/String;ZILjava/lang/Object;)Lkotlinx/serialization/json/JsonElement;

build.gradle.kts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,35 @@ subprojects {
7575

7676
kotlin {
7777
sourceSets {
78-
commonMain.dependencies {
79-
implementation(libs.kotlinxSerialization)
80-
implementation(libs.kotlinReflect)
78+
commonMain {
79+
dependencies {
80+
implementation(libs.kotlinxSerialization)
81+
implementation(libs.kotlinReflect)
82+
}
83+
jvm {
84+
compilerOptions {
85+
jvmTarget = JvmTarget.JVM_1_8
86+
// We set this in addition to jvmTarget, as it gives us some additional verification that
87+
// we don't use more modern JDK features:
88+
// https://kotlinlang.org/docs/compiler-reference.html#xjdk-release-version
89+
freeCompilerArgs.add("-Xjdk-release=1.8")
90+
}
91+
}
8192
}
82-
commonTest.dependencies {
83-
implementation(libs.kotlinTest)
84-
implementation(libs.kotest)
93+
commonTest {
94+
dependencies {
95+
implementation(libs.kotlinTest)
96+
implementation(libs.kotest)
97+
}
98+
// kotest 6 requires JDK 11
99+
jvm {
100+
compilerOptions {
101+
jvmTarget = JvmTarget.JVM_11
102+
freeCompilerArgs.add("-Xjdk-release=11")
103+
}
104+
}
85105
}
106+
86107
jvmMain.dependencies {
87108
implementation(libs.slf4j)
88109
implementation(libs.jackson)
@@ -97,12 +118,6 @@ kotlin {
97118
}
98119

99120
jvm {
100-
compilerOptions {
101-
jvmTarget = JvmTarget.JVM_1_8
102-
// Needed due to https://youtrack.jetbrains.com/issue/KT-49746
103-
freeCompilerArgs.add("-Xjdk-release=1.8")
104-
}
105-
106121
// To use JUnit 5 rather than JUnit 4:
107122
// https://kotlinlang.org/docs/gradle-configure-project.html#jvm-variants-of-kotlin-test
108123
testRuns["test"].executionTask.configure { useJUnitPlatform() }

src/commonMain/kotlin/dev/hermannm/devlog/LogBuilder.kt

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33

44
package dev.hermannm.devlog
55

6+
import kotlin.reflect.KType
7+
import kotlin.reflect.typeOf
68
import kotlinx.serialization.SerializationStrategy
9+
import kotlinx.serialization.serializer
710

811
/**
912
* Class used in the logging methods on [Logger], allowing you to add structured key-value data to
@@ -103,11 +106,55 @@ internal constructor(
103106
* - `java.math.BigDecimal`
104107
*/
105108
public inline fun <reified ValueT> field(key: String, value: ValueT) {
106-
encodeFieldValue(
107-
value,
108-
onJson = { jsonValue -> logEvent.addJsonField(key, jsonValue) },
109-
onString = { stringValue -> logEvent.addStringField(key, stringValue) },
110-
)
109+
try {
110+
addLogFieldOfType(key, value, typeOf<ValueT>())
111+
} catch (_: Exception) {
112+
// Falls back to `toString()` if serialization fails
113+
addStringLogField(key, value)
114+
}
115+
}
116+
117+
/**
118+
* We split this out from [field] to reduce the amount of inlined code (more inlined code ->
119+
* bigger code size -> possibly worse performance, and also more internal API that must be exposed
120+
* with `@PublishedApi`).
121+
*/
122+
@PublishedApi
123+
internal fun addLogFieldOfType(
124+
key: String,
125+
value: Any?,
126+
valueType: KType,
127+
) {
128+
when {
129+
/** See [JSON_NULL_VALUE] for why we handle nulls like this. */
130+
value == null -> {
131+
logEvent.addJsonField(key, JSON_NULL_VALUE)
132+
}
133+
// Special case for String, to avoid redundant serialization
134+
value is String -> {
135+
logEvent.addStringField(key, value)
136+
}
137+
// Special case for common types that kotlinx.serialization doesn't handle by default
138+
fieldValueShouldUseToString(value) -> {
139+
logEvent.addStringField(key, value.toString())
140+
}
141+
// Try to serialize with kotlinx.serialization - if it fails, we fall back to toString below
142+
else -> {
143+
val serializer = LOG_FIELD_JSON_FORMAT.serializersModule.serializer(valueType)
144+
val serializedValue = LOG_FIELD_JSON_FORMAT.encodeToString(serializer, value)
145+
logEvent.addJsonField(key, serializedValue)
146+
}
147+
}
148+
}
149+
150+
/** `toString()` fallback for the log field value when [addLogFieldOfType] fails. */
151+
@PublishedApi
152+
internal fun addStringLogField(key: String, value: Any?) {
153+
if (value == null) {
154+
logEvent.addJsonField(key, JSON_NULL_VALUE)
155+
} else {
156+
logEvent.addStringField(key, value.toString())
157+
}
111158
}
112159

113160
/**
@@ -181,12 +228,17 @@ internal constructor(
181228
value: ValueT?,
182229
serializer: SerializationStrategy<ValueT>
183230
) {
184-
encodeFieldValueWithSerializer(
185-
value,
186-
serializer,
187-
onJson = { jsonValue -> logEvent.addJsonField(key, jsonValue) },
188-
onString = { stringValue -> logEvent.addStringField(key, stringValue) },
189-
)
231+
try {
232+
if (value == null) {
233+
logEvent.addJsonField(key, JSON_NULL_VALUE)
234+
} else {
235+
val serializedValue = LOG_FIELD_JSON_FORMAT.encodeToString(serializer, value)
236+
logEvent.addJsonField(key, serializedValue)
237+
}
238+
} catch (_: Exception) {
239+
// Falls back to `toString()` if serialization fails
240+
addStringLogField(key, value)
241+
}
190242
}
191243

192244
/**

src/commonMain/kotlin/dev/hermannm/devlog/LogField.kt

Lines changed: 72 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package dev.hermannm.devlog
55

6+
import kotlin.reflect.KType
7+
import kotlin.reflect.typeOf
68
import kotlinx.serialization.ExperimentalSerializationApi
79
import kotlinx.serialization.SerializationStrategy
810
import kotlinx.serialization.json.Json
@@ -15,6 +17,7 @@ import kotlinx.serialization.json.JsonUnquotedLiteral
1517
import kotlinx.serialization.json.booleanOrNull
1618
import kotlinx.serialization.json.doubleOrNull
1719
import kotlinx.serialization.json.longOrNull
20+
import kotlinx.serialization.serializer
1821

1922
/**
2023
* A log field is a key-value pair for adding structured data to logs.
@@ -43,7 +46,6 @@ import kotlinx.serialization.json.longOrNull
4346
* 3. Logging context fields (from [withLoggingContext])
4447
*/
4548
public class LogField
46-
@PublishedApi
4749
internal constructor(
4850
@kotlin.jvm.JvmField internal val key: String,
4951
@kotlin.jvm.JvmField internal val value: String,
@@ -94,11 +96,55 @@ internal constructor(
9496
* - `java.math.BigDecimal`
9597
*/
9698
public inline fun <reified ValueT> field(key: String, value: ValueT): LogField {
97-
return encodeFieldValue(
98-
value,
99-
onJson = { jsonValue -> LogField(key, jsonValue, isJson = true) },
100-
onString = { stringValue -> LogField(key, stringValue, isJson = false) },
101-
)
99+
return try {
100+
createLogFieldOfType(key, value, valueType = typeOf<ValueT>())
101+
} catch (_: Exception) {
102+
// Falls back to `toString()` if serialization fails
103+
createStringLogField(key, value)
104+
}
105+
}
106+
107+
/**
108+
* We split this out from [field] to reduce the amount of inlined code (more inlined code -> bigger
109+
* code size -> possibly worse performance, and also more internal API that must be exposed with
110+
* `@PublishedApi`).
111+
*/
112+
@PublishedApi
113+
internal fun createLogFieldOfType(
114+
key: String,
115+
value: Any?,
116+
valueType: KType,
117+
): LogField {
118+
return when {
119+
/** See [JSON_NULL_VALUE] for why we handle nulls like this. */
120+
value == null -> {
121+
LogField(key, JSON_NULL_VALUE, isJson = true)
122+
}
123+
// Special case for String, to avoid redundant serialization
124+
value is String -> {
125+
LogField(key, value, isJson = false)
126+
}
127+
// Special case for common types that kotlinx.serialization doesn't handle by default
128+
fieldValueShouldUseToString(value) -> {
129+
LogField(key, value.toString(), isJson = false)
130+
}
131+
// Try to serialize with kotlinx.serialization - if it fails, we fall back to toString below
132+
else -> {
133+
val serializer = LOG_FIELD_JSON_FORMAT.serializersModule.serializer(valueType)
134+
val serializedValue = LOG_FIELD_JSON_FORMAT.encodeToString(serializer, value)
135+
LogField(key, serializedValue, isJson = true)
136+
}
137+
}
138+
}
139+
140+
/** `toString()` fallback for the log field value when [createLogFieldOfType] fails. */
141+
@PublishedApi
142+
internal fun createStringLogField(key: String, value: Any?): LogField {
143+
return if (value == null) {
144+
LogField(key, JSON_NULL_VALUE, isJson = true)
145+
} else {
146+
LogField(key, value.toString(), isJson = false)
147+
}
102148
}
103149

104150
/**
@@ -134,70 +180,16 @@ public fun <ValueT : Any> field(
134180
value: ValueT?,
135181
serializer: SerializationStrategy<ValueT>,
136182
): LogField {
137-
return encodeFieldValueWithSerializer(
138-
value,
139-
serializer,
140-
onJson = { jsonValue -> LogField(key, jsonValue, isJson = true) },
141-
onString = { stringValue -> LogField(key, stringValue, isJson = false) },
142-
)
143-
}
144-
145-
/**
146-
* Encodes the given value to JSON, calling [onJson] with the result. If we failed to encode to
147-
* JSON, or the value was already a string (or one of the types with special handling as explained
148-
* on [field]'s docstring), we fall back to its `toString` representation and call [onString].
149-
*
150-
* We take callbacks for the different results here instead of returning a return value. This is
151-
* because we use this in both [field] and [LogBuilder.field], and they want to do different things
152-
* with the encoded value:
153-
* - [field] constructs a [LogField] with it
154-
* - [LogBuilder.field] passes the value to [LogEvent.addStringField] or [LogEvent.addJsonField]
155-
*
156-
* If we used a return value here, we would have to wrap it in an object to convey whether it was
157-
* encoded to JSON or just a plain string, which requires an allocation. By instead taking callbacks
158-
* and making the function `inline`, we pay no extra cost.
159-
*/
160-
@PublishedApi
161-
internal inline fun <reified ValueT, ReturnT> encodeFieldValue(
162-
value: ValueT,
163-
crossinline onJson: (String) -> ReturnT,
164-
crossinline onString: (String) -> ReturnT,
165-
): ReturnT {
166-
try {
167-
return when {
168-
value == null -> onJson(JSON_NULL_VALUE)
169-
// Special case for String, to avoid redundant serialization
170-
value is String -> onString(value)
171-
// Special case for common types that kotlinx.serialization doesn't handle by default
172-
fieldValueShouldUseToString(value) -> onString(value.toString())
173-
// Try to serialize with kotlinx.serialization - if it fails, we fall back to toString below
174-
else -> onJson(LOG_FIELD_JSON_FORMAT.encodeToString(value))
175-
}
176-
} catch (_: Exception) {
177-
// We don't want to ever throw an exception from constructing a log field, which may happen if
178-
// serialization fails, for example. So in these cases we fall back to toString()
179-
return onString(value.toString())
180-
}
181-
}
182-
183-
/** Same as [encodeFieldValue], but takes a user-provided serializer for serializing the value. */
184-
internal inline fun <ValueT : Any, ReturnT> encodeFieldValueWithSerializer(
185-
value: ValueT?,
186-
serializer: SerializationStrategy<ValueT>,
187-
crossinline onJson: (String) -> ReturnT,
188-
crossinline onString: (String) -> ReturnT,
189-
): ReturnT {
190-
try {
191-
return when {
192-
// Handle nulls here, so users don't have to deal with passing a null-handling serializer
193-
value == null -> onJson(JSON_NULL_VALUE)
194-
// Try to serialize with kotlinx.serialization - if it fails, we fall back to toString below
195-
else -> onJson(LOG_FIELD_JSON_FORMAT.encodeToString(serializer, value))
183+
return try {
184+
if (value == null) {
185+
LogField(key, JSON_NULL_VALUE, isJson = true)
186+
} else {
187+
val serializedValue = LOG_FIELD_JSON_FORMAT.encodeToString(serializer, value)
188+
LogField(key, serializedValue, isJson = true)
196189
}
197190
} catch (_: Exception) {
198-
// We don't want to ever throw an exception from constructing a log field, which may happen if
199-
// serialization fails, for example. So in these cases we fall back to toString().
200-
return onString(value.toString())
191+
// Falls back to `toString()` if serialization fails
192+
createStringLogField(key, value)
201193
}
202194
}
203195

@@ -324,8 +316,16 @@ public fun rawJson(json: String, validJson: Boolean = false): JsonElement {
324316
* Validates that the given raw JSON string is valid JSON, calling [onValidJson] if it is, or
325317
* [onInvalidJson] if it's not.
326318
*
327-
* We take lambdas here instead of returning a value, for the same reason as [encodeFieldValue]. We
328-
* use this in both [rawJsonField] and [LogBuilder.rawJsonField].
319+
* We take callbacks for the different results here instead of returning a return value. This is
320+
* because we use this in both [rawJsonField] and [LogBuilder.rawJsonField], and they want to do
321+
* different things with the encoded value:
322+
* - [rawJsonField] constructs a [LogField] with it
323+
* - [LogBuilder.rawJsonField] passes the value to [LogEvent.addStringField] or
324+
* [LogEvent.addJsonField]
325+
*
326+
* If we used a return value here, we would have to wrap it in an object to convey whether it was
327+
* encoded to JSON or just a plain string, which requires an allocation. By instead taking callbacks
328+
* and making the function `inline`, we pay no extra cost.
329329
*/
330330
internal inline fun <ReturnT> validateRawJson(
331331
json: String,
@@ -405,7 +405,7 @@ internal fun isValidJson(jsonElement: JsonElement): Boolean {
405405
* We make this an expect-actual function, so that implementations can use platform-specific types
406406
* (such as Java standard library classes on the JVM).
407407
*/
408-
@PublishedApi internal expect fun fieldValueShouldUseToString(value: Any): Boolean
408+
internal expect fun fieldValueShouldUseToString(value: Any): Boolean
409409

410410
/**
411411
* SLF4J supports null values in `KeyValuePair`s, and it's up to the logger implementation for how
@@ -414,9 +414,8 @@ internal fun isValidJson(jsonElement: JsonElement): Boolean {
414414
* field was omitted due to some error. So in this library, we instead use a JSON `null` as the
415415
* value for null log fields.
416416
*/
417-
@PublishedApi internal const val JSON_NULL_VALUE: String = "null"
417+
internal const val JSON_NULL_VALUE: String = "null"
418418

419-
@PublishedApi
420419
@kotlin.jvm.JvmField
421420
internal val LOG_FIELD_JSON_FORMAT: Json = Json {
422421
encodeDefaults = true

0 commit comments

Comments
 (0)