Skip to content

Commit

Permalink
Improve performance of Tracer.invoke() function
Browse files Browse the repository at this point in the history
Improve performance of `Tracer.invoke()`:
- ~70% improvement if there are no trace event consumers
- ~55% improvement if there are trace event consumers

Changes:
* Introduce caching of trace event method annotations in the `Tracer.invoke()` function.
* Skip creation of a `TraceEvent` object if there are no event consumers. This saves us from performing an expensive
  `LocalDateTime.now()` call.
* Skip retrieval of trace event method annotations for simple log function invocations, if there are no event consumers.
* Remove string concatenations in `createLogMessage()` functions. Only use `StringBuilder` functions.
* Change `toStringRegistry` to use `item::javaClass.name` as key, and use `item::class.toString()` only as fallback.
* Skip reading `item.javaClass.getMethod("toString").declaringClass` in `convertToStringUsingRegistry()`: the
  `item.toString()` function of `Any` will anyway print the name of the class.
* Fix execution of unit tests requiring mockk-jvm (`extensions` and `traceevents` modules): no unit test for these
  modules was being run anymore, after the 1.8.2 update.
* Fix the `IfExtensionsTest` unit tests: these tests were broken by mockk 1.13.4 (due to
  mockk/mockk#1033).
  • Loading branch information
OresteSalerno-TomTom committed Jul 6, 2023
1 parent a332a59 commit 129dce8
Show file tree
Hide file tree
Showing 10 changed files with 260 additions and 124 deletions.
15 changes: 15 additions & 0 deletions README.md
Expand Up @@ -659,6 +659,21 @@ Contributors: Timon Kanters, Jeroen Erik Jensen, Krzysztof Karczewski

## Release notes

### 1.8.3

* Introduce caching of trace event method annotations in the `Tracer.invoke()` function.
* Skip creation of a `TraceEvent` object if there are no event consumers. This saves us from performing an expensive
`LocalDateTime.now()` call.
* Skip retrieval of trace event method annotations for simple log function invocations, if there are no event consumers.
* Remove string concatenations in `createLogMessage()` functions. Only use `StringBuilder` functions.
* Change `toStringRegistry` to use `item::javaClass.name` as key, and use `item::class.toString()` only as fallback.
* Skip reading `item.javaClass.getMethod("toString").declaringClass` in `convertToStringUsingRegistry()`: the
`item.toString()` function of `Any` will anyway print the name of the class.
* Fix execution of unit tests requiring mockk-jvm (`extensions` and `traceevents` modules): no unit test for these
modules was being run anymore, after the 1.8.2 update.
* Fix the `IfExtensionsTest` unit tests: these tests were broken by mockk 1.13.4 (due to
https://github.com/mockk/mockk/issues/1033).

### 1.8.2

* Dependencies updated.
Expand Down
2 changes: 1 addition & 1 deletion extensions/pom.xml
Expand Up @@ -24,7 +24,7 @@
<parent>
<groupId>com.tomtom.kotlin</groupId>
<artifactId>kotlin-tools</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<artifactId>extensions</artifactId>
Expand Down
Expand Up @@ -16,6 +16,7 @@

package com.tomtom.kotlin.extensions

import io.mockk.every
import io.mockk.spyk
import io.mockk.verify
import kotlin.test.assertEquals
Expand All @@ -28,7 +29,10 @@ internal class IfExtensionsTest {
fun `ifTrue on true`() {
// GIVEN
val sut = true
val ifTrueBlock = spyk({ 1 })
// TODO: replace with `spyk({ 1 })` when https://github.com/mockk/mockk/issues/1033 is fixed.
val ifTrueBlock: () -> Int = spyk {
every { this@spyk.invoke() } answers { 1 }
}

// WHEN
val result = sut.ifTrue(ifTrueBlock)
Expand All @@ -42,7 +46,10 @@ internal class IfExtensionsTest {
fun `ifTrue on false`() {
// GIVEN
val sut = false
val ifTrueBlock = spyk({ 1 })
// TODO: replace with `spyk({ 1 })` when https://github.com/mockk/mockk/issues/1033 is fixed.
val ifTrueBlock: () -> Int = spyk {
every { this@spyk.invoke() } answers { 1 }
}

// WHEN
val result = sut.ifTrue(ifTrueBlock)
Expand All @@ -56,7 +63,10 @@ internal class IfExtensionsTest {
fun `ifTrue on null`() {
// GIVEN
val sut: Boolean? = null
val ifTrueBlock = spyk({ 1 })
// TODO: replace with `spyk({ 1 })` when https://github.com/mockk/mockk/issues/1033 is fixed.
val ifTrueBlock: () -> Int = spyk {
every { this@spyk.invoke() } answers { 1 }
}

// WHEN
val result = sut.ifTrue(ifTrueBlock)
Expand Down Expand Up @@ -90,7 +100,10 @@ internal class IfExtensionsTest {
fun `ifNull on null`() {
// GIVEN
val sut: Int? = null
val ifNullBlock = spyk({ 1 })
// TODO: replace with `spyk({ 1 })` when https://github.com/mockk/mockk/issues/1033 is fixed.
val ifNullBlock: () -> Int = spyk {
every { this@spyk.invoke() } answers { 1 }
}

// WHEN
val result = sut.ifNull(ifNullBlock)
Expand All @@ -108,7 +121,10 @@ internal class IfExtensionsTest {
fun `ifNull with different types`() {
// GIVEN
val sut = A
val ifNullBlock = spyk({ B })
// TODO: replace with `spyk({ B })` when https://github.com/mockk/mockk/issues/1033 is fixed.
val ifNullBlock: () -> Base = spyk {
every { this@spyk.invoke() } answers { B }
}

// WHEN
val result: Base = sut.ifNull(ifNullBlock)
Expand Down
2 changes: 1 addition & 1 deletion memoization/pom.xml
Expand Up @@ -24,7 +24,7 @@
<parent>
<groupId>com.tomtom.kotlin</groupId>
<artifactId>kotlin-tools</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<artifactId>memoization</artifactId>
Expand Down
11 changes: 9 additions & 2 deletions pom.xml
Expand Up @@ -22,7 +22,7 @@

<groupId>com.tomtom.kotlin</groupId>
<artifactId>kotlin-tools</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
<packaging>pom</packaging>

<name>Kotlin Tools</name>
Expand Down Expand Up @@ -100,7 +100,7 @@
<maven-gpg-plugin.version>3.0.1</maven-gpg-plugin.version>
<maven-project-info-reports-plugin.version>3.4.2</maven-project-info-reports-plugin.version>
<maven-source-plugin.version>3.2.1</maven-source-plugin.version>
<maven-surefire-plugin.version>2.22.2</maven-surefire-plugin.version>
<maven-surefire-plugin.version>3.1.2</maven-surefire-plugin.version>
<nexus-staging-maven-plugin.version>1.6.8</nexus-staging-maven-plugin.version>

<!-- Library versions. -->
Expand Down Expand Up @@ -184,6 +184,13 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven-surefire-plugin.version}</version>
<dependencies>
<dependency>
<groupId>org.apache.maven.surefire</groupId>
<artifactId>surefire-junit4</artifactId>
<version>3.1.2</version>
</dependency>
</dependencies>
</plugin>

<plugin>
Expand Down
2 changes: 1 addition & 1 deletion traceevents/pom.xml
Expand Up @@ -24,7 +24,7 @@
<parent>
<groupId>com.tomtom.kotlin</groupId>
<artifactId>kotlin-tools</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<artifactId>traceevents</artifactId>
Expand Down
Expand Up @@ -66,7 +66,7 @@ public interface TraceLog {
message: String,
e: Throwable?
) = "[${LocalDateTime.now().format(DateTimeFormatter.ISO_DATE_TIME)}] $logLevel " +
"$tag: $message${e?.let { ", ${Tracer.formatThrowable(it, true)}" } ?: ""}"
"$tag: $message${e?.let { ", ${Tracer.formatThrowable(it)}" } ?: ""}"

private var theLogger: Logger = DefaultLoggerToStdout
}
Expand Down

0 comments on commit 129dce8

Please sign in to comment.