-
Notifications
You must be signed in to change notification settings - Fork 18
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
Convert internal serdes from beans to ordinary classes #761
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there some JMH benchmarks that show the improvement in startup time vs the old way?
serde-support/src/main/java/io/micronaut/serde/support/serdes/InstantSerde.java
Show resolved
Hide resolved
I don't see much of the improvement. But having fewer beans might help. |
serde-support/src/main/java/io/micronaut/serde/support/DefaultSerdeRegistry.java
Outdated
Show resolved
Hide resolved
|
.iterator(); | ||
// pick the bean with the highest priority | ||
final Iterator<BeanDefinition<T>> i = candidates.stream() | ||
.sorted((bean1, bean2) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Comparator.comparingInt
@Singleton | ||
@Primary | ||
@BootstrapContextCompatible | ||
@Internal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
breaking change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are added as beans in LegacyBeansFactory
This should improve the startup.
I have extracted all of the serdes defined in the
DefaultSerdeRegistry
into own package.