Skip to content

Conversation

sdeleuze
Copy link
Contributor

ObjectMapper instantiation is costly, so unless its usage is one-shot, it is better to create a reusable instance for upcoming usage.

Also, before this commit, serialization of most Kotlin classes was not supported due to the lack of proper Jackson KotlinModule detection.

This commit:

  • Avoids per invocation ObjectMapper instantiation when relevant
  • Automatically detects and enables well-known Jackson modules including the Kotlin one
  • Removes org.springframework.ai.vectorstore.JsonUtils which looks not needed anymore

More optimizations are possible like reusing more ObjectMapper instances, but this could introduce more breaking changes so this commit intends to be a good first step.

Kotlin tests will be provided in a follow-up PR.

@tzolov tzolov self-assigned this Oct 23, 2024
@tzolov tzolov added this to the 1.0.0-M4 milestone Oct 23, 2024
ObjectMapper instantiation is costly, so unless its usage
is one-shot, it is better to create a reusable instance
for upcoming usage.

Also, before this commit, serialization of most Kotlin
classes was not supported due to the lack of proper
Jackson KotlinModule detection.

This commit:
 - Avoids per invocation ObjectMapper instantiation when
   relevant
 - Automatically detects and enables well-known Jackson
   modules including the Kotlin one
 - Removes org.springframework.ai.vectorstore.JsonUtils
   which looks not needed anymore

More optimizations are possible like reusing more
ObjectMapper instances, but this could introduce more breaking
changes so this commit intends to be a good first step.

Kotlin tests will be provided in a follow-up commit.
@tzolov
Copy link
Contributor

tzolov commented Oct 24, 2024

Thanks @sdeleuze ,
LGTM. there few small changes I will apply with the merge.

@tzolov
Copy link
Contributor

tzolov commented Oct 24, 2024

Few additional changes (below) where applied, then rebased, squashed and merged at: f21b8a4

Additional changes:

  • Update ModelOptionsUtils to use JacksonUtils.instantiateAvailableModules()
  • Add missing license headers
  • Add missing author Javadoc comments

@tzolov tzolov closed this Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants