RUST-871 Support direct serialization to BSON bytes #279

patrickfreed · 2021-07-22T20:31:08Z

This PR introduces the to_writer and to_vec methods, which can serialize a generic T directly to BSON bytes without going through Document. This should enable substantial performance improvements in the driver, as well as be generally useful in the BSON library.

As part of testing this feature, I enabled decimal128 serde testing on evergreen (RUST-446). decimal128 tests were originally added to evergreen in #279.

patrickfreed · 2021-07-22T20:31:41Z

.evergreen/config.yml

@@ -92,16 +92,6 @@ functions:
          ${PREPARE_SHELL}
          .evergreen/run-tests-u2i.sh

-  "run serde tests":


rather than the serde tests being their own task in evergreen, I updated each of the other tasks to run it too with their associated feature flags on.

patrickfreed · 2021-07-22T20:36:17Z

src/ser/mod.rs

@@ -83,6 +89,24 @@ fn write_f128<W: Write + ?Sized>(writer: &mut W, val: Decimal128) -> Result<()>
    writer.write_all(&raw).map_err(From::from)
 }

+#[inline]
+fn write_binary<W: Write>(mut writer: W, bytes: &[u8], subtype: BinarySubtype) -> Result<()> {


this is just copied from below

patrickfreed · 2021-07-22T20:37:33Z

src/ser/raw/mod.rs

@@ -0,0 +1,458 @@
+mod document_serializer;


I recommend starting the review in this file and then branching out to the others as you encounter them here.

patrickfreed · 2021-07-22T20:59:06Z

src/ser/raw/mod.rs

+    }
+
+    fn serialize_struct(self, name: &'static str, _len: usize) -> Result<Self::SerializeStruct> {
+        let value_type = match name {


BSON types which don't fit into serde's data model (e.g. ObjectId) are serialized as structs matching the extended JSON format for that type. The ValueSerializer is then in charge of interpreting the extended JSON format and producing actual BSON from it. The regular serializer works this way too, but it goes from Bson to Bson rather than serde data model -> BSON.

The "name" of the struct is used to incidate which BSON type, and the values are basically just the BSON type's extended JSON wrapper name. The only divergence is we also use $codeWithScope to differentiate from just a regular $code with no scope.

If the name doesn't match any of the special strings, the struct will just be serialized as a document.

patrickfreed · 2021-07-22T21:02:13Z

src/bson.rs

@@ -698,6 +698,20 @@ impl Bson {
                }
            }

+            ["$numberDecimalBytes"] => {


Unintuitively, this function is used in the existing serializer to convert between the intermediate serialized form (i.e. and "extended document") and the final serialized form. Now that decimal128 can serialize this way, this needed to be updated to interpret it.

patrickfreed · 2021-07-22T21:04:45Z

src/ser/raw/value_serializer.rs

+
+/// A serializer used specifically for serializing the serde-data-model form of a BSON type (e.g.
+/// `Binary`) to raw bytes.
+pub(crate) struct ValueSerializer<'a> {


This type is used to interpret any extended JSON formatted types in serde's data model and serialize them to actual BSON. It keeps track of where it's at via a state machine.

patrickfreed · 2021-07-22T21:10:01Z

src/ser/raw/value_serializer.rs

+    type Ok = ();
+    type Error = Error;
+
+    fn serialize_field<T: ?Sized>(&mut self, key: &'static str, value: &T) -> Result<()>


this method handles all the state transitions.

src/ser/serde.rs

patrickfreed · 2021-07-22T21:15:14Z

src/ser/serde.rs

@@ -551,8 +607,12 @@ impl Serialize for Decimal128 {
    where
        S: ser::Serializer,
    {
-        let value = Bson::Decimal128(self.clone());
-        value.serialize(serializer)
+        let mut state = serializer.serialize_struct("$numberDecimal", 1)?;


When the decimal128 flag was enabled, this used to produce actual extended JSON format (i.e. { "$numberDecimal": <decimal as a string> }), but per the bug we discussed in triage, the current implementation's to_string is lossy. For that reason, it seems safer to produce this unique format that does round trip properly until we have a complete decimal128 implementation.

For users relying on the output of this, their code will break, but that's probably better than them silently losing data.
Also note that we classify the decimal128 feature flag as "experimental" and subject to breaking changes, so this is within our contract. Code that omits the flag will not break.

patrickfreed · 2021-07-22T21:16:13Z

src/tests/spec/corpus.rs

@@ -61,63 +61,95 @@ fn run_test(test: TestFile) {

        let canonical_bson = hex::decode(&valid.canonical_bson).expect(&description);

-        let bson_to_native_cb =


I went ahead and updated the naming scheme of the different things here. Let me know if this makes sense. I know the old one got the point of being unreadable to me at least, so hopefully this is better.

src/ser/raw/mod.rs

src/ser/raw/value_serializer.rs

src/ser/serde.rs

src/ser/mod.rs

patrickfreed added 19 commits July 12, 2021 18:59

initial impl working

8a4d9f6

corpus, code w scope not working

82fb936

wip custom stuff

123a102

datetime binary oid symbol done

67b195d

checkpoint

c15cb01

wip

586038d

corpus working

9e5894a

move doc serializer to own file

db62358

move value serializer to its own file

48143e8

various cleanup

a8595b1

wip serde tests

b041228

fix serde tests

88aeaeb

lossless decimal128 serialization via bytes

efe2867

various cleanup

d9ccb01

add u2i tests to serde-tests

7b9eb51

add docstrings

3617b48

cleanup

acfd4bc

unify binary serialization impls

d2b2212

inline everything

4ce6913

patrickfreed commented Jul 22, 2021

View reviewed changes

patrickfreed marked this pull request as ready for review July 22, 2021 21:19

patrickfreed requested review from isabelatkinson and abr-egn July 22, 2021 21:19

fix fmt in serde-tests

a55c519

abr-egn reviewed Jul 23, 2021

View reviewed changes

src/ser/raw/mod.rs Show resolved Hide resolved

src/ser/raw/mod.rs Show resolved Hide resolved

src/ser/raw/value_serializer.rs Show resolved Hide resolved

src/ser/serde.rs Show resolved Hide resolved

src/ser/mod.rs Outdated Show resolved Hide resolved

remove bson::to_writer

f512590

abr-egn approved these changes Jul 27, 2021

View reviewed changes

isabelatkinson approved these changes Jul 28, 2021

View reviewed changes

patrickfreed merged commit 50621ff into mongodb:master Jul 28, 2021

patrickfreed mentioned this pull request Jul 29, 2021

RUST-871 Serialize directly to BSON bytes in insert operations mongodb/mongo-rust-driver#406

Merged

This was referenced Aug 2, 2021

RUST-942 Generate 5 random bytes instead of 3 for ObjectIds #284

Merged

RUST-960 Remove the decimal128 feature flag #287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RUST-871 Support direct serialization to BSON bytes #279

RUST-871 Support direct serialization to BSON bytes #279

patrickfreed commented Jul 22, 2021 •

edited

Loading

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

patrickfreed Jul 22, 2021

		@@ -61,63 +61,95 @@ fn run_test(test: TestFile) {

		let canonical_bson = hex::decode(&valid.canonical_bson).expect(&description);

		let bson_to_native_cb =

RUST-871 Support direct serialization to BSON bytes #279

RUST-871 Support direct serialization to BSON bytes #279

Conversation

patrickfreed commented Jul 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickfreed commented Jul 22, 2021 •

edited

Loading