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

feat(kotlin): Add unsigned array support and tests for arrays and strings #1891

Merged
merged 10 commits into from
Oct 19, 2024

Conversation

wywen
Copy link
Contributor

@wywen wywen commented Oct 18, 2024

What does this PR do?

This PR adds tests for serializing/deserializing:

  • Strings (same as Java)
  • Primitive arrays in Kotlin (same as Java)
  • Array in kotlin (same as Java)
  • Unsigned arrays in kotlin - UByteArray, UIntArray, UShortArray, ULongArray

Unsigned arrays in kotlin are currently marked experimental, and are subject to API changes (hence the annotations needed to suppress those warnings).

These types are implemented as a view over the signed arrays e.g. UByteArray is a view over ByteArray with contents reinterpreted as UByte, so serializers. The current implementation delegate to existing serializers for corresponding signed types.

The xlang type id is set to LIST for unsigned types.

Related issues

#683

Does this PR introduce any user-facing change?

Yes. Unsigned primitives no longer need to be registered for fury-kotlin.

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

N/A

@wywen wywen changed the title feat (kotlin): Add unsigned array support and tests for arrays and strings feat(kotlin): Add unsigned array support and tests for arrays and strings Oct 18, 2024
}

override fun write(buffer: MemoryBuffer, value: T) {
val delegatingSerializer = fury.classResolver.getSerializer(delegateClass)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this as a field of the DelegatingArraySerializer to reduce lookup cost?

Copy link
Collaborator

@chaokunyang chaokunyang Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess all those unsiged array type are final

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup those arrays are final! I made it a lazily initialized field.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lazy can be removed too

@chaokunyang chaokunyang merged commit 750a511 into apache:main Oct 19, 2024
37 checks passed
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.

2 participants