Skip to content
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.

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

Improve performance of Tracer.invoke() function #52

Merged
merged 2 commits into from Jul 6, 2023

Conversation

OresteSalerno-TomTom
Copy link
Collaborator

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 Broken spying on functions mockk/mockk#1033).

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).
@OresteSalerno-TomTom
Copy link
Collaborator Author

One note about the changes to the flow of the invoke() function: I have already tried a more elegant solution involving the use of lazy variables, but that had a worse performance impact than this solution. Instantiation of Lazy objects is not free.

@RijnBuve-TomTom RijnBuve-TomTom merged commit d33416c into master Jul 6, 2023
1 check passed
@delete-merged-branch delete-merged-branch bot deleted the improve-tracer-performance branch July 6, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants