After this change, `mini_table` only has MiniTable definitions themselves. Everything having to do with the MiniDescriptor wire format is in `mini_descriptor`.
Also rearranged some of the files in mini_table to have better structure for `internal/`.
This CL contains no functional change.
PiperOrigin-RevId: 543529112
The `kUpb_DecodeOption_ExperimentalAllowUnlinked` flag to the decoder will enable the new behavior. When that flag is not passed, tree shaking with the old model will still be possible.
"Dynamic tree shaking" in upb is a feature that allows messages to be parsed even if the MiniTables have not been fully linked. Unlinked sub-message fields can be parsed by preserving their data in the unknown fields. If the application later discovers that the message field is actually needed, the MiniTable can be patched to properly link that field, and existing message instances can "promote" the data from the unknown fields to an actual message of the correct type.
Before this change, dynamic tree shaking stored unparsed message data in the unknown fields of the *parent*. In effect, we were treating the field as if it did not exist at all. This meant that parsing an unlinked field did not affect the hasbits or oneof cases of the parent, nor did it create a `upb_Array` or `upb_Map` for array/map fields. Only when a message was linked and promoted did any of these things occur.
While this model had some amount of conceptual simplicity, it caused significant problems with oneofs. When multiple fields inside a single oneof are parsed from the wire, order matters, because later oneof fields must overwrite earlier ones. Dynamic tree shaking can mean that some fields in a oneof are linked while others are not. It is essential that we preserve this ordering semantic even when dynamic tree shaking is being used, but it is difficult to do if the oneof's data can be split between linked fields (which have been reified into parsed field data) and unlinked fields (whose data lives in the unknown fields of the parent).
To solve this problem, this CL changes the representation for unlinked fields. Instead of being placed in the parent's unknown fields, we create an actual message instance for each unlinked message we parse, but we use a placeholder "empty message" MiniTable as the message's type. All of the message's data will therefore be placed into the "empty message's" unknown fields. But unlike before, this "empty message" is actually present according to the hasbits, oneof case, and `upb_Array`/`upb_Map` of the parent. This means that all of the oneof presence logic works as normal.
Since the MiniTable can be patched at any time, we need a bit in the message instance itself to signal whether a pointer to a sub-message is an "empty message" or not. When dynamic tree shaking is in use, all users must be capable of recognizing an empty message and acting accordingly (promoting, etc) even if the MiniTable itself says that the field is linked.
Because dynamic tree shaking imposes this extra requirement on users, we require that users pass an extra option to the decoder to allow parsing of unlinked sub-messages. Many existing users of upb (Ruby, PHP, Python, etc) will always have fully-linked MiniTables, so there is no reason for them to add extra logic to handle empty messages. By omitting the `kUpb_DecodeOption_ExperimentalAllowUnlinked` option, they will be relieved of the duty to check the tagged pointer that would indicate an empty, unlinked message.
For existing users of dynamic tree shaking, there are three main changes:
1. The APIs in message/promote.h have changed, and users will need to update to the new interfaces.
2. The model for maps has changed slightly. Before, we required that map entries always had their values linked; for dynamic tree shaking to apply to maps, we required that the *entry* was left unlinked, not the entry's value. In the new model, that is reversed: map entries must always be linked, but a map entry's value can be unlinked.
3. The presence model for unlinked fields has changed. Unlinked fields will now register as "present" from the perspective of hasbits, oneof cases, and array/map entries. Users must test the tagged pointer to know if a message is of the correct, linked type or whether it is a placeholder "empty" message. There is a new function `upb_Message_GetTaggedMessagePtr()`, as well as a new accessor `upb_MessageValue.tagged_msg_val` that can be used to read and test the tagged pointer directly.
PiperOrigin-RevId: 535288031
The fields of upb_MiniTableField are intended to be internal-only, accessed only through public functions like `upb_MiniTable_GetSubMessageTable()`. But over time, clients have started accessing many of these fields directly. This is an easy mistake to make, as there is no clear signal that the fields should not be used in applications. This makes the implementation difficult to change without breaking users.
The new `UPB_PRIVATE()` macro appends an unpredictable string to each private symbol. This makes it very difficult to accidentally use a private symbol, since users would need to write something like `field->submsg_index_dont_copy_me__upb_internal_use_only`. This is still possible to do, but it leaves a clear wart in the code showing that an an encapsulation break has occurred. The `UPB_PRIVATE()` macro itself is defined in `port/def.inc`, which users cannot include directly.
Once we land this, more such CLs will follow for the other fields of `upb_MiniTable*`. We will add inline functions as needed to provide the semantic functionality needed by users.
PiperOrigin-RevId: 523166901
It no longer has any users. If we need it later, we can add it back.
This saves one pointer of memory from `sizeof(upb_Arena)`.
Also, we now allow fuses if if the block allocators differ. This is made possible by cl/520144430, but was not take advantage of in that CL.
PiperOrigin-RevId: 520174588
To correctly handle this case we must add the serialized map entry to the unknown fields. Ideally we could merely preserve the map entry's serialized bytes from the input. However this is tricky to do if we are streaming and the previous buffer where the map entry began is no longer available.
This CL fixes this edge case by using the encoder to re-encode the map entry rather than using the input bytes directly.
While this fix is reasonably simple and reliable, it has two unfortunate properties. One is performance: we now must run the encoder to recreate bytes that we already saw in the input.
The other is dependencies: this fix has the unfortunate property of making the decoder depend on the encoder. In applications that only want the decoder but not the encoder, this will increase binary size. But the practical effects of this are probably minimal (the vast majority of applications that depend on the decoder will also use the encoder).
We can revisit this later and see if there is a better way of preserving the input bytes without re-encoding. For now this fix is simple and correct and fixes the fuzz bug.
PiperOrigin-RevId: 505381927
The initial motivation for this CL was to fix a bug found by fuzzing. But the fuzz bug pointed out a few edge cases that this CL corrects:
1. The core bug is that we were allowing a map entry sub-message to be linked to a group field. This is not allowed in protobuf schemas, but we did not check for this edge case in `upb_MiniTable_SetSubMessage()`, so we were de facto allowing it. This triggered some bad behavior in the parser whereby we pushed a limit without checking its validity first.
2. To defend against this, I added asserts in `upb_MiniTable_SetSubMessage()` to verify the type of the field we are linking, to ensure that a group field is not linked to a map entry sub-message. But this should probably be changed to return an error instead of relying on asserts for this.
3. I changed the fuzz util code that builds the MiniTable so that it will never violate this new invariant. The fuzz util code now can run into situations where a group field has no valid non-map-entry sub-message to select. In those cases it will simply not register any sub-message for that field.
4. Previously group did not support leaving sub-messages unregistered. Previously I added this feature for sub-messages but not for groups. There is no reason why dynamic tree shaking should not work for group fields, so I extended the feature to support groups also.
PiperOrigin-RevId: 504913630
According to https://en.cppreference.com/w/c/program/setjmp automatic variables
modified in a function calling setjmp can have indeterminate values. Instead,
refactor all functions calling setjmp so that the function calling setjmp
doesn’t have any local variables.
Part VI: Wire encoder/decoder.
PiperOrigin-RevId: 504810940
The overall motivation for this interface is to consolidate many places in upb that are parsing wire format data directly.
This interface is not yet complete, but this is a good start. We have enough to port the wire format parsing in accessors.c to this interface. We can follow up by porting more places that do wire format parsing.
PiperOrigin-RevId: 498109788
Moving the logic down to EpsCopyInputStream makes it easier to test and reuse this functionality.
We also implement aliasing for the final bytes of the patch buffer, which has never been supported before. We used to always force a copy for any data parsed out of the patch buffer at the end of the stream.
Much of this logic is ported directly from the C++ EpsCopyInputStream class.
PiperOrigin-RevId: 498091644
This mirrors the structure of C++ protobuf, which has an EpsCopyInputStream class.
This will lay the foundation for making EpsCopyInputStream capable of true streaming, by reading its input from a ZeroCopyInputStream. It also lets us test EpsCopyInputStream separately from the decoder: see the new unit test that fuzzes upb_EpsCopyInputStream.
After this CL is submitted, the two decoders (the normal decoder and the fast decoder) should no longer be accessing the members of upb_EpsCopyInputStream.
PiperOrigin-RevId: 494400285
This required some work to unify map entry messages with regular messages, with respect to presence. Before map entry fields could never have presence. Now they can have presence according to normal rules. Note that this only applies to times that the user constructs a map entry directly.
PiperOrigin-RevId: 490611656
Prior to this CL, there were several different code paths for reading/writing message data. Generated code, MiniTable accessors, and reflection all performed direct manipulation of the bits and bytes in a message, but they all had distinct implementations that did not share much of any code. This divergence meant that they could easily have different behavior, bugs could creep into one but not another, and we would need three different sets of tests to get full test coverage. This also made it very difficult to change the internal representation in any way, since it would require updating many places in the code.
With this CL, the three different APIs for accessing message data now all share a common set of functions. The common functions all take a `upb_MiniTableField` as the canonical description of a field's type and layout. The lowest-level functions are very branchy, as they must test for every possible variation in the field type (field vs oneof, hasbit vs no-hasbit, different field sizes, whether a nonzero default value exists, extension vs. regular field), however these functions are declared inline and designed to be very optimizable when values are known at compile time.
In generated accessors, for example, we can declare constant `upb_MiniTableField` instances so that all values can constant-propagate, and we can get fully specialized code even though we are calling a generic function. On the other hand, when we use the generic functions from reflection, we get runtime branches since values are not known at compile time. But even the function is written to still be as efficient as possible even when used from reflection. For example, we use memcpy() calls with constant length so that the compiler can optimize these into inline loads/stores without having to make an out-of-line call to memcpy().
In this way, this CL should be a benefit to both correctness and performance. It will also make it easier to change the message representation, for example to optimize the encoder by giving hasbits to all fields.
Note that we have not completely consolidated all access in this CL:
1. Some functions outside of get/set such as clear and hazzers are not yet unified.
2. The encoder and decoder still touch the message without going through the common functions. The encoder and decoder require a bit more specialized code to get good performance when reading/writing fields en masse.
PiperOrigin-RevId: 490016095
Remove circular dependencies that were bouncing back and forth between
msg_internal.h and mini_table/, including:
- splitting out each mini table subtype into its own header
- moving the non-reflection message code into message/
- moving the accessors from mini_table/ to message/
PiperOrigin-RevId: 489121042
- replace all instances of the deprecated iterator with the much nicer new one
- fix a bug which caused the new iterator to skip all values in the hash array
- fix a bug which caused the new iterator to skip the first value in the hash table
- delete the old iterator code
- also replace most uses of the deprecated string hash table iterator
PiperOrigin-RevId: 489093240
There are several other functions which might eventually end up here and ideally become unified across json/ and text/ and io/ so this is just a first step to create the new subdir and get rid of upb/internal/
PiperOrigin-RevId: 488954926
Move the map-related functions from msg_internal.h that are only used in generated code into map_gencode_util.h. Then move the rest of the map-related functions from msg_internal.h into map_internal.h.
PiperOrigin-RevId: 486299140
Jason Lunn
Joshua Haberman
Eric Salo
Deanna Garcia
Protobuf Team Bot
Mike Kruskal