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

[DRAFT] Add MinimalGenerics and allow users to set a GenericsStrategy #879

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

theigl
Copy link
Collaborator

@theigl theigl commented Jan 26, 2022

We currently support wo strategies for generics: DefaultGenerics and NoGenerics.

The former is enabled by default, but was responsible for most of the bugs found in Kryo 5 and incurs some performance overhead. Users can currently opt-out of generics optimization by setting Kryo.setOptimizeGenerics(false). Internally, this flag sets the generic strategy to NoGenerics.

This PR introduces a third strategy called MinimalGenerics that only processes generic type information, but does not attempt to resolve type variables. This makes sense because the type variable logic is quite involved, still has edge-case issues (e.g. #864), and causes most of the performance overhead related to generics handling.

The new strategy is safer and should theoretically be faster than disabling generics optimizations altogether, because concrete generic types can still be resolved. CollectionSerializer and MapSerializer should especially benefit from this.

TODO

  • JavaDoc for strategies and configuration option
  • Find a better name for new generics strategy

@theigl theigl self-assigned this Jan 26, 2022
@theigl theigl changed the title [DRAFT] Add MinimalGenerics and allow users to set a GenericStrategy [DRAFT] Add MinimalGenerics and allow users to set a GenericsStrategy Jan 26, 2022
@NathanSweet
Copy link
Member

Ideally full generics would be efficient and not have bugs for edge cases (at least #864 is resolved). If the performance overhead can't be improved then I'm fine having a MinimalGenerics implementation that is roughly what we had in Kryo 4. Though IIRC the performance difference from Kryo 4 to 5 was only marginally in 4's favor. Note that using generics information can remove the need to write types in many cases, so the trade with minimal/no generics is less CPU for larger serialized size.

@NathanSweet
Copy link
Member

BTW I got here from the V6 ideas page. All the other ideas sound good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants