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

serialization refactoring #691

Merged
merged 23 commits into from
Oct 1, 2024
Merged

Conversation

DenisBiryukov91
Copy link
Contributor

@DenisBiryukov91 DenisBiryukov91 commented Sep 25, 2024

serialization refactoring
Related to eclipse-zenoh/zenoh#1472

Copy link

PR missing one of the required labels: {'internal', 'enhancement', 'documentation', 'breaking-change', 'dependencies', 'bug', 'new feature'}

include/zenoh-pico/config.h Outdated Show resolved Hide resolved
examples/unix/.DS_Store Outdated Show resolved Hide resolved
@milyin
Copy link
Contributor

milyin commented Sep 26, 2024

  • Do we really need z_bytes_writer_serialize_sequence_end ? Seems like it's excessive
  • I think we need to return error when sequence of elements of some type is not finished and attempt to write element of different type is made
  • Do we support sequence of sequences?

examples/unix/c11/z_bytes.c Outdated Show resolved Hide resolved
z_string_from_str(&kvs_input[1].value, "def", NULL, NULL);

z_bytes_writer_t writer = z_bytes_get_writer(z_loan_mut(payload));
z_bytes_writer_serialize_sequence_begin(&writer, 2);
Copy link
Member

Choose a reason for hiding this comment

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

Why sequence but not e.g. array or list?

Copy link
Contributor Author

@DenisBiryukov91 DenisBiryukov91 Sep 26, 2024

Choose a reason for hiding this comment

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

Because it can be used for any sequence/container (i.e. list/array/set/map etc). Mostly to choose something neutral to have something similar to iterate notion.

z_bytes_reader_deserialize_sequence_begin(&reader, &num_elements1);
assert(num_elements1 == 3);
for (size_t j = 0; j < 3; ++j) {
z_bytes_reader_deserialize_uint64(&reader, &u);
Copy link
Member

Choose a reason for hiding this comment

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

What will happens there if we try deserialize more elements that was serialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deserialize function calls will return a error if we run out of bytes to read, otherwise there are no any checks so it is up to the user to ensure he serializes/deserializes data correctly.

Copy link
Member

Choose a reason for hiding this comment

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

We definitely need to provide at least some protection or checks, but this can be done later, the main thing is that we have a returned error code, which we can then expand.

z_bytes_reader_deserialize_uint64(&reader, &u);
assert(u == cs.u[i][j]);
}
z_bytes_reader_deserialize_sequence_end(&reader);
Copy link
Member

Choose a reason for hiding this comment

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

What will happens there if we try deserialize less elements that was serialized?

Copy link
Contributor Author

@DenisBiryukov91 DenisBiryukov91 Sep 26, 2024

Choose a reason for hiding this comment

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

nothing, there is no fast way to check because user can serialize tuples which would involve more serialize calls than number of elements.

examples/unix/c11/z_get_attachment.c Show resolved Hide resolved
examples/unix/c11/z_get_attachment.c Outdated Show resolved Hide resolved
Co-authored-by: Alexander Bushnev <sashacmc@gmail.com>
@DenisBiryukov91
Copy link
Contributor Author

  • Do we really need z_bytes_writer_serialize_sequence_end ? Seems like it's excessive
  • I think we need to return error when sequence of elements of some type is not finished and attempt to write element of different type is made
  • Do we support sequence of sequences?

As discussed we do not need z_bytes_writer_serialize_sequence_end now, but might require it for future optimizations.

Yes we support sequence of sequences.

add ze_serializer_from/to_writer and ze_deserializer_from/to_reader;
@milyin milyin merged commit 1c73e0c into eclipse-zenoh:main Oct 1, 2024
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Things could work better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants