Re: [protobuf] Protobuf v3.12.0-rc1 is released

[Nadav Samet]

The need for non-empty oneof came up in conversations with users. The other
evidence for this being a real user need is that PGV has a validator for
non-empty oneofs  in
their code gen, so it wouldn't be unreasonable if someone implemented this
via reflection rather than codegen.

Totally understand that this may be the safest way to roll this change out.
This is a pretty significant change to proto3, so it might be worth adding
to the changelog the motivation for this change and considerations that
were taken (i.e. possibly wrapper types are not good enough due to the
extra space)

On Tue, May 5, 2020 at 6:50 PM Josh Haberman  wrote:

> On Tue, May 5, 2020 at 6:24 PM Nadav Samet  wrote:
>
>> What representations do you mean? In general it is not expected that
>>> oneof names are exposed in wire formats. Authors of .proto files are free
>>> to change them at will without breaking the wire.
>>>
>>
>> An example would be code that produces an alternative JSON representation
>> that groups all the fields of a oneof inside a nested object. For example
>> if a message like
>> message MyMessage {
>>   oneof foo {
>> int32 v = 1
>>   }
>> }
>>
>> gets encoded to JSON by user's code as:
>> "my_one_of": {"foo": 5}}
>>
>> Then such code would have to be modified to treat synthetic
>> oneofs differently.
>>
>
> Can you point to code that does this, or is it hypothetical?
>
> I agree that if such a format existed, serialization code would need to
> change to use the "real_" variants of the oneof accessors. But the parsers
> and serializers for all of the real formats we use with reflection
> (protobuf binary, TextFormat, JSON) did not need to change to support
> proto3 optional, which is a significant benefit. I suspect lots of other
> reflection-based algorithms will benefit from this too.
>
>
>> Can you give an example of some of the code that would be affected?
>>>
>>
>> Some users use oneofs to build ADTs, and for them oneofs should never be
>> empty . To enforce this
>> at runtime, they may have written a validator that takes an instance of a
>> message, and uses its descriptor to traverse all the oneofs and fail when
>> it finds a oneof where none of its fields is set. With the proposed change,
>> this validator would have to be changed to only care about non-synthetic
>> oneofs.
>>
>
> This sounds like it is also hypothetical? I think it's helpful to talk
> about real examples when possible.
>
> It's true that in some cases code will need to change to ignore synthetic
> oneofs. But in the cases where we've had to implement this, it is not a
> very intrusive change. It mainly consists of two simple transformations:
>
> 1. f->containing_oneof() -> f->real_containing_oneof()
> 2. f->oneof_decl_count() -> f->real_oneof_decl_count()
>
> I agree it would have been nicer not to have to use synthetic oneofs. But
> there is a *lot* of code out there that manipulates protos using
> reflection. Synthetic oneofs allow many/most of these algorithms to work
> with proto3 optional fields with no modifications. Without them it would be
> a riskier change to roll out, because we don't know how many algorithms
> would simply not be aware that proto3 optional fields exist and need to
> preserve presence.
>


-- 
-Nadav

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CANZcNEoewchNP9qLNAsVRmhQUbbMnzpC0APjjU9EDw80myB2RQ%40mail.gmail.com.

Re: [protobuf] Re: Discrepancy in generated code of unsafe_arena_release_foo

[King Kong]

As a related issue, I remember reading some comments in protobuf source 
code that says google internal protobuf supports arena allocation that's 
not portable (probably assumes certain std::string implementation), thus it 
is not open sourced, what's the plan for string arena allocation support? 
If I don't care about portability, is it possible to get the non-portable 
solution somehow?

On Tuesday, May 5, 2020 at 7:02:39 PM UTC-7, Josh Haberman wrote:
>
> On Tue, May 5, 2020 at 4:20 PM King Kong > 
> wrote:
>
>> Thanks for your reply.
>>
>> Yes, I have been using arena, but I wanted to do better than batching 
>> heap allocation, when possible. The "scoped leasing" use case I described 
>> above achieve 0 heap allocation all the time. I guess my code could use 
>> GetArena() at runtime to decide how the "leasing" object is created and 
>> which set of set APIs to use.
>>
>
> If you want to avoid any heap allocation, you could give the arena an 
> initial block of memory from the stack.
>
> If you are trying to optimize, keep in mind that most of the savings from 
> arenas come at deallocation time, where arena-allocated protos can simply 
> skip the destructor. If you are using heap-allocated protos, the destructor 
> has to check every message field to see if it needs to be deleted.
>
> Arenas have given such performance benefits that we are focused on that 
> use case. If we loosen the checks in unsafe_arena functions, it could force 
> us to compromise later to keep supporting the heap-allocated case. I don't 
> think we want to loosen these restrictions.
>  
>
>> On Tuesday, May 5, 2020 at 2:25:05 PM UTC-7, Josh Haberman wrote:
>>>
>>> The documentation for unsafe_arena_release_foo() says:
>>>
>>> > This method should only be used when the parent message is on the 
>>> arena and the submessage is on the same arena, or an arena with equivalent 
>>> lifetime.
>>>
>>> The DCHECK() you highlighted is checking the first part of this 
>>> requirement.
>>>
>>> Stack/heap-allocated messages must always uniquely own their 
>>> sub-messages, which must always be on the heap. Protobuf doesn't support 
>>> the use case you are describing, where a message temporarily points to a 
>>> message it doesn't own. If we allowed that, protobuf itself would crash if 
>>> a message in this state was ever destroyed.
>>>
>>> If you are trying to avoid heap allocation overhead, the recommended 
>>> solution is arenas.
>>>
>>> On Tuesday, May 5, 2020 at 12:44:32 PM UTC-7, King Kong wrote:

 I am using unsafe_arena_set_allocated_* and unsafe_arena_release_* APIs 
 to avoid
 unnecessary allocations when the sub-objects are only used in a 
 function scope,
 unsafe_arena_release_* is always called before leaving the scope. At 
 the beginning,

 I chose to always use the arena versions of unsafe APIs because the 
 non-arena version 

 unsafe_release does extra allocation when the object is allocated on 
 arena, and the arena

 version does not check if the object is allocated in arena, based on 
 generated code like 

 the following:


 inline foo* outter1::unsafe_arena_release_foo() {
 foo* temp = foo_;
 foo_ = nullptr;
 return temp;
 }


 However, I later also saw the following the generated code with arena 
 check, it means it is not okay to call for heap-allocated objects.

 inline void outter2::unsafe_arena_set_allocated_bar(
 std::string* bar) {
 GOOGLE_DCHECK(GetArenaNoVirtual() != nullptr);
 ... ...
 }


 So, why the discrepancy?


 The official documentation at 
 https://developers.google.com/protocol-buffers/docs/reference/arenas says 
 the correct set of APIs should be used

 according to how to the object is allocated, but having two different 
 sets of unsafe APIs makes it harder to use, and it makes the code less 
 robust to future changes. I am wondering if protobuf owners also realize 
 such an issue and start to remove arena checking gradually? It that the 
 case? I would like to hear how others think.


 Thanks.

>>> -- 
>> You received this message because you are subscribed to a topic in the 
>> Google Groups "Protocol Buffers" group.
>> To unsubscribe from this topic, visit 
>> https://groups.google.com/d/topic/protobuf/C26jc_jrCJQ/unsubscribe.
>> To unsubscribe from this group and all its topics, send an email to 
>> prot...@googlegroups.com .
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/protobuf/bfa10586-05af-430c-b809-5e4c65d43b5c%40googlegroups.com
>>  
>> 
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from 

Re: [protobuf] Re: Discrepancy in generated code of unsafe_arena_release_foo

['Josh Haberman' via Protocol Buffers]

On Tue, May 5, 2020 at 4:20 PM King Kong  wrote:

> Thanks for your reply.
>
> Yes, I have been using arena, but I wanted to do better than batching heap
> allocation, when possible. The "scoped leasing" use case I described above
> achieve 0 heap allocation all the time. I guess my code could use
> GetArena() at runtime to decide how the "leasing" object is created and
> which set of set APIs to use.
>

If you want to avoid any heap allocation, you could give the arena an
initial block of memory from the stack.

If you are trying to optimize, keep in mind that most of the savings from
arenas come at deallocation time, where arena-allocated protos can simply
skip the destructor. If you are using heap-allocated protos, the destructor
has to check every message field to see if it needs to be deleted.

Arenas have given such performance benefits that we are focused on that use
case. If we loosen the checks in unsafe_arena functions, it could force us
to compromise later to keep supporting the heap-allocated case. I don't
think we want to loosen these restrictions.


> On Tuesday, May 5, 2020 at 2:25:05 PM UTC-7, Josh Haberman wrote:
>>
>> The documentation for unsafe_arena_release_foo() says:
>>
>> > This method should only be used when the parent message is on the arena
>> and the submessage is on the same arena, or an arena with equivalent
>> lifetime.
>>
>> The DCHECK() you highlighted is checking the first part of this
>> requirement.
>>
>> Stack/heap-allocated messages must always uniquely own their
>> sub-messages, which must always be on the heap. Protobuf doesn't support
>> the use case you are describing, where a message temporarily points to a
>> message it doesn't own. If we allowed that, protobuf itself would crash if
>> a message in this state was ever destroyed.
>>
>> If you are trying to avoid heap allocation overhead, the recommended
>> solution is arenas.
>>
>> On Tuesday, May 5, 2020 at 12:44:32 PM UTC-7, King Kong wrote:
>>>
>>> I am using unsafe_arena_set_allocated_* and unsafe_arena_release_* APIs
>>> to avoid
>>> unnecessary allocations when the sub-objects are only used in a function
>>> scope,
>>> unsafe_arena_release_* is always called before leaving the scope. At the
>>> beginning,
>>>
>>> I chose to always use the arena versions of unsafe APIs because the
>>> non-arena version
>>>
>>> unsafe_release does extra allocation when the object is allocated on
>>> arena, and the arena
>>>
>>> version does not check if the object is allocated in arena, based on
>>> generated code like
>>>
>>> the following:
>>>
>>>
>>> inline foo* outter1::unsafe_arena_release_foo() {
>>> foo* temp = foo_;
>>> foo_ = nullptr;
>>> return temp;
>>> }
>>>
>>>
>>> However, I later also saw the following the generated code with arena
>>> check, it means it is not okay to call for heap-allocated objects.
>>>
>>> inline void outter2::unsafe_arena_set_allocated_bar(
>>> std::string* bar) {
>>> GOOGLE_DCHECK(GetArenaNoVirtual() != nullptr);
>>> ... ...
>>> }
>>>
>>>
>>> So, why the discrepancy?
>>>
>>>
>>> The official documentation at
>>> https://developers.google.com/protocol-buffers/docs/reference/arenas says
>>> the correct set of APIs should be used
>>>
>>> according to how to the object is allocated, but having two different
>>> sets of unsafe APIs makes it harder to use, and it makes the code less
>>> robust to future changes. I am wondering if protobuf owners also realize
>>> such an issue and start to remove arena checking gradually? It that the
>>> case? I would like to hear how others think.
>>>
>>>
>>> Thanks.
>>>
>> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Protocol Buffers" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/protobuf/C26jc_jrCJQ/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> protobuf+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/protobuf/bfa10586-05af-430c-b809-5e4c65d43b5c%40googlegroups.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CAFFe09gqZJJ40xvdbTNRqQ9%3DaJYxud%3Dtyufg-reYajTOT%2BPDsw%40mail.gmail.com.

Re: [protobuf] Protobuf v3.12.0-rc1 is released

['Josh Haberman' via Protocol Buffers]

On Tue, May 5, 2020 at 6:24 PM Nadav Samet  wrote:

> What representations do you mean? In general it is not expected that oneof
>> names are exposed in wire formats. Authors of .proto files are free to
>> change them at will without breaking the wire.
>>
>
> An example would be code that produces an alternative JSON representation
> that groups all the fields of a oneof inside a nested object. For example
> if a message like
> message MyMessage {
>   oneof foo {
> int32 v = 1
>   }
> }
>
> gets encoded to JSON by user's code as:
> "my_one_of": {"foo": 5}}
>
> Then such code would have to be modified to treat synthetic
> oneofs differently.
>

Can you point to code that does this, or is it hypothetical?

I agree that if such a format existed, serialization code would need to
change to use the "real_" variants of the oneof accessors. But the parsers
and serializers for all of the real formats we use with reflection
(protobuf binary, TextFormat, JSON) did not need to change to support
proto3 optional, which is a significant benefit. I suspect lots of other
reflection-based algorithms will benefit from this too.


> Can you give an example of some of the code that would be affected?
>>
>
> Some users use oneofs to build ADTs, and for them oneofs should never be
> empty . To enforce this at
> runtime, they may have written a validator that takes an instance of a
> message, and uses its descriptor to traverse all the oneofs and fail when
> it finds a oneof where none of its fields is set. With the proposed change,
> this validator would have to be changed to only care about non-synthetic
> oneofs.
>

This sounds like it is also hypothetical? I think it's helpful to talk
about real examples when possible.

It's true that in some cases code will need to change to ignore synthetic
oneofs. But in the cases where we've had to implement this, it is not a
very intrusive change. It mainly consists of two simple transformations:

1. f->containing_oneof() -> f->real_containing_oneof()
2. f->oneof_decl_count() -> f->real_oneof_decl_count()

I agree it would have been nicer not to have to use synthetic oneofs. But
there is a *lot* of code out there that manipulates protos using
reflection. Synthetic oneofs allow many/most of these algorithms to work
with proto3 optional fields with no modifications. Without them it would be
a riskier change to roll out, because we don't know how many algorithms
would simply not be aware that proto3 optional fields exist and need to
preserve presence.

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CAFFe09jO-Fpqup4k_U%3DKkpkzDTqD1YOpYiOpt37S6xxdo3Rs9w%40mail.gmail.com.

[protobuf] Re: Discrepancy in generated code of unsafe_arena_release_foo

[King Kong]

I could be wrong, but it seems to arena checking is only done for 
std::string in generated unsafe_arena_* code. I wonder if it is because 
google internal version of protobuf has arena support for std::string, thus 
the special treatment.

On Tuesday, May 5, 2020 at 5:09:01 PM UTC-7, King Kong wrote:
>
> unsafe_arena_set_allcoated* and unsafe_arena_release* API are "unsafe" 
> APIs, callers should know what they are doing.
> Why not remove arena checking in all generated 
> unsafe_arena_release*  code? This will make unsafe_arena* APIs work for 
> both arena-allocated and heap allocated objects. It seems a straightforward 
> change to me, arena checking in unsafe_arena_release*  does not always 
> happen anyway.
>
> On Tuesday, May 5, 2020 at 4:20:33 PM UTC-7, King Kong wrote:
>>
>> Thanks for your reply.
>>
>> Yes, I have been using arena, but I wanted to do better than batching 
>> heap allocation, when possible. The "scoped leasing" use case I described 
>> above achieve 0 heap allocation all the time. I guess my code could use 
>> GetArena() at runtime to decide how the "leasing" object is created and 
>> which set of set APIs to use.
>>
>> On Tuesday, May 5, 2020 at 2:25:05 PM UTC-7, Josh Haberman wrote:
>>>
>>> The documentation for unsafe_arena_release_foo() says:
>>>
>>> > This method should only be used when the parent message is on the 
>>> arena and the submessage is on the same arena, or an arena with equivalent 
>>> lifetime.
>>>
>>> The DCHECK() you highlighted is checking the first part of this 
>>> requirement.
>>>
>>> Stack/heap-allocated messages must always uniquely own their 
>>> sub-messages, which must always be on the heap. Protobuf doesn't support 
>>> the use case you are describing, where a message temporarily points to a 
>>> message it doesn't own. If we allowed that, protobuf itself would crash if 
>>> a message in this state was ever destroyed.
>>>
>>> If you are trying to avoid heap allocation overhead, the recommended 
>>> solution is arenas.
>>>
>>> On Tuesday, May 5, 2020 at 12:44:32 PM UTC-7, King Kong wrote:

 I am using unsafe_arena_set_allocated_* and unsafe_arena_release_* APIs 
 to avoid
 unnecessary allocations when the sub-objects are only used in a 
 function scope,
 unsafe_arena_release_* is always called before leaving the scope. At 
 the beginning,

 I chose to always use the arena versions of unsafe APIs because the 
 non-arena version 

 unsafe_release does extra allocation when the object is allocated on 
 arena, and the arena

 version does not check if the object is allocated in arena, based on 
 generated code like 

 the following:


 inline foo* outter1::unsafe_arena_release_foo() {
 foo* temp = foo_;
 foo_ = nullptr;
 return temp;
 }


 However, I later also saw the following the generated code with arena 
 check, it means it is not okay to call for heap-allocated objects.

 inline void outter2::unsafe_arena_set_allocated_bar(
 std::string* bar) {
 GOOGLE_DCHECK(GetArenaNoVirtual() != nullptr);
 ... ...
 }


 So, why the discrepancy?


 The official documentation at 
 https://developers.google.com/protocol-buffers/docs/reference/arenas says 
 the correct set of APIs should be used

 according to how to the object is allocated, but having two different 
 sets of unsafe APIs makes it harder to use, and it makes the code less 
 robust to future changes. I am wondering if protobuf owners also realize 
 such an issue and start to remove arena checking gradually? It that the 
 case? I would like to hear how others think.


 Thanks.

>>>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/52f265e3-595a-4b58-ac2c-9544026557b9%40googlegroups.com.

Re: [protobuf] Protobuf v3.12.0-rc1 is released

[Nadav Samet]

>
> What representations do you mean? In general it is not expected that oneof
> names are exposed in wire formats. Authors of .proto files are free to
> change them at will without breaking the wire.
>

An example would be code that produces an alternative JSON representation
that groups all the fields of a oneof inside a nested object. For example
if a message like
message MyMessage {
  oneof foo {
int32 v = 1
  }
}

gets encoded to JSON by user's code as:
"my_one_of": {"foo": 5}}

Then such code would have to be modified to treat synthetic
oneofs differently.


> Can you give an example of some of the code that would be affected?
>

Some users use oneofs to build ADTs, and for them oneofs should never be
empty . To enforce this at
runtime, they may have written a validator that takes an instance of a
message, and uses its descriptor to traverse all the oneofs and fail when
it finds a oneof where none of its fields is set. With the proposed change,
this validator would have to be changed to only care about non-synthetic
oneofs.

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CANZcNEojEGQ4KES%3D7cd%2BCpkJ-Qs_3EBKOTpZ_c_JdLH7SOHg1Q%40mail.gmail.com.

Re: [protobuf] Protobuf v3.12.0-rc1 is released

['Josh Haberman' via Protocol Buffers]

Hi Nadav,

On Tue, May 5, 2020 at 5:25 PM Nadav Samet  wrote:

> Hi Josh, thanks for the heads up on the upcoming change needed by source
> code generators.
>
> At first sight, the implementation detail around
> synthetic/real oneofs seems like a clever hack to avoid breaking some code
> that uses reflection, but the consequence would be that:
> 1. some code that does reflection would still break and need to be
> updated, for example if the reflection translates from a representation
> that depends on the oneof's name.
>

What representations do you mean? In general it is not expected that oneof
names are exposed in wire formats. Authors of .proto files are free to
change them at will without breaking the wire.


> 2. the synthetic/real oneofs distinction is going to remain with us in the
> foreseeable future and we'll have to keep it in mind when we write
> generators and reflection code: we will always need to think which list of
> oneofs is the appropriate one to iterate on.
>

Reflection code should not generally have to think about the difference
between real and synthetic oneofs. The main exception is if you are
generating an API or documentation that is consumed by users. But I expect
this is a small minority of all code using reflection.

Can you give an example of some of the code that would be affected?


> -Nadav
>
>
> On Tue, May 5, 2020 at 2:13 PM 'Josh Haberman' via Protocol Buffers <
> protobuf@googlegroups.com> wrote:
>
>> I have released Protobuf v3.12.0-rc1 to GitHub and the per-language
>> repositories (NuGet, npm, RubyGems, etc).
>>
>> https://github.com/protocolbuffers/protobuf/releases/tag/v3.12.0-rc1
>>
>> Please try it out and let us know if you have any issues. The full 3.12.0
>> release should follow within a week or so.
>>
>> If you own a code generator, please especially take note of the changelog
>> in the link above. A new experimental feature (proto3 presence) require all
>> code generators to update.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Protocol Buffers" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to protobuf+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/protobuf/41207f02-8d37-42e4-a148-6580c90ab48a%40googlegroups.com
>> 
>> .
>>
>
>
> --
> -Nadav
>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CAFFe09ikXoeRbxVoK23L660dcq6PO3zQqXsp3GgPEmsTdKez1w%40mail.gmail.com.

Re: [protobuf] Protobuf v3.12.0-rc1 is released

[Nadav Samet]

Hi Josh, thanks for the heads up on the upcoming change needed by source
code generators.

At first sight, the implementation detail around
synthetic/real oneofs seems like a clever hack to avoid breaking some code
that uses reflection, but the consequence would be that:
1. some code that does reflection would still break and need to be updated,
for example if the reflection translates from a representation that depends
on the oneof's name.
2. the synthetic/real oneofs distinction is going to remain with us in the
foreseeable future and we'll have to keep it in mind when we write
generators and reflection code: we will always need to think which list of
oneofs is the appropriate one to iterate on.

-Nadav


On Tue, May 5, 2020 at 2:13 PM 'Josh Haberman' via Protocol Buffers <
protobuf@googlegroups.com> wrote:

> I have released Protobuf v3.12.0-rc1 to GitHub and the per-language
> repositories (NuGet, npm, RubyGems, etc).
>
> https://github.com/protocolbuffers/protobuf/releases/tag/v3.12.0-rc1
>
> Please try it out and let us know if you have any issues. The full 3.12.0
> release should follow within a week or so.
>
> If you own a code generator, please especially take note of the changelog
> in the link above. A new experimental feature (proto3 presence) require all
> code generators to update.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Protocol Buffers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to protobuf+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/protobuf/41207f02-8d37-42e4-a148-6580c90ab48a%40googlegroups.com
> 
> .
>


-- 
-Nadav

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/CANZcNEpHdp04Smku3kV1WEeHNzYaAeNSF-ABpiY3MXi7oPeiqw%40mail.gmail.com.

[protobuf] Re: Discrepancy in generated code of unsafe_arena_release_foo

[King Kong]

Thanks for your reply.

Yes, I have been using arena, but I wanted to do better than batching heap 
allocation, when possible. The "scoped leasing" use case I described above 
achieve 0 heap allocation all the time. I guess my code could use 
GetArena() at runtime to decide how the "leasing" object is created and 
which set of set APIs to use.

On Tuesday, May 5, 2020 at 2:25:05 PM UTC-7, Josh Haberman wrote:
>
> The documentation for unsafe_arena_release_foo() says:
>
> > This method should only be used when the parent message is on the arena 
> and the submessage is on the same arena, or an arena with equivalent 
> lifetime.
>
> The DCHECK() you highlighted is checking the first part of this 
> requirement.
>
> Stack/heap-allocated messages must always uniquely own their sub-messages, 
> which must always be on the heap. Protobuf doesn't support the use case you 
> are describing, where a message temporarily points to a message it doesn't 
> own. If we allowed that, protobuf itself would crash if a message in this 
> state was ever destroyed.
>
> If you are trying to avoid heap allocation overhead, the recommended 
> solution is arenas.
>
> On Tuesday, May 5, 2020 at 12:44:32 PM UTC-7, King Kong wrote:
>>
>> I am using unsafe_arena_set_allocated_* and unsafe_arena_release_* APIs 
>> to avoid
>> unnecessary allocations when the sub-objects are only used in a function 
>> scope,
>> unsafe_arena_release_* is always called before leaving the scope. At the 
>> beginning,
>>
>> I chose to always use the arena versions of unsafe APIs because the 
>> non-arena version 
>>
>> unsafe_release does extra allocation when the object is allocated on 
>> arena, and the arena
>>
>> version does not check if the object is allocated in arena, based on 
>> generated code like 
>>
>> the following:
>>
>>
>> inline foo* outter1::unsafe_arena_release_foo() {
>> foo* temp = foo_;
>> foo_ = nullptr;
>> return temp;
>> }
>>
>>
>> However, I later also saw the following the generated code with arena 
>> check, it means it is not okay to call for heap-allocated objects.
>>
>> inline void outter2::unsafe_arena_set_allocated_bar(
>> std::string* bar) {
>> GOOGLE_DCHECK(GetArenaNoVirtual() != nullptr);
>> ... ...
>> }
>>
>>
>> So, why the discrepancy?
>>
>>
>> The official documentation at 
>> https://developers.google.com/protocol-buffers/docs/reference/arenas says 
>> the correct set of APIs should be used
>>
>> according to how to the object is allocated, but having two different 
>> sets of unsafe APIs makes it harder to use, and it makes the code less 
>> robust to future changes. I am wondering if protobuf owners also realize 
>> such an issue and start to remove arena checking gradually? It that the 
>> case? I would like to hear how others think.
>>
>>
>> Thanks.
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/bfa10586-05af-430c-b809-5e4c65d43b5c%40googlegroups.com.

[protobuf] Re: Discrepancy in generated code of unsafe_arena_release_foo

['Josh Haberman' via Protocol Buffers]

The documentation for unsafe_arena_release_foo() says:

> This method should only be used when the parent message is on the arena 
and the submessage is on the same arena, or an arena with equivalent 
lifetime.

The DCHECK() you highlighted is checking the first part of this requirement.

Stack/heap-allocated messages must always uniquely own their sub-messages, 
which must always be on the heap. Protobuf doesn't support the use case you 
are describing, where a message temporarily points to a message it doesn't 
own. If we allowed that, protobuf itself would crash if a message in this 
state was ever destroyed.

If you are trying to avoid heap allocation overhead, the recommended 
solution is arenas.

On Tuesday, May 5, 2020 at 12:44:32 PM UTC-7, King Kong wrote:
>
> I am using unsafe_arena_set_allocated_* and unsafe_arena_release_* APIs to 
> avoid
> unnecessary allocations when the sub-objects are only used in a function 
> scope,
> unsafe_arena_release_* is always called before leaving the scope. At the 
> beginning,
>
> I chose to always use the arena versions of unsafe APIs because the 
> non-arena version 
>
> unsafe_release does extra allocation when the object is allocated on 
> arena, and the arena
>
> version does not check if the object is allocated in arena, based on 
> generated code like 
>
> the following:
>
>
> inline foo* outter1::unsafe_arena_release_foo() {
> foo* temp = foo_;
> foo_ = nullptr;
> return temp;
> }
>
>
> However, I later also saw the following the generated code with arena 
> check, it means it is not okay to call for heap-allocated objects.
>
> inline void outter2::unsafe_arena_set_allocated_bar(
> std::string* bar) {
> GOOGLE_DCHECK(GetArenaNoVirtual() != nullptr);
> ... ...
> }
>
>
> So, why the discrepancy?
>
>
> The official documentation at 
> https://developers.google.com/protocol-buffers/docs/reference/arenas says 
> the correct set of APIs should be used
>
> according to how to the object is allocated, but having two different sets 
> of unsafe APIs makes it harder to use, and it makes the code less robust to 
> future changes. I am wondering if protobuf owners also realize such an 
> issue and start to remove arena checking gradually? It that the case? I 
> would like to hear how others think.
>
>
> Thanks.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/b20963a8-6852-495e-be89-dc93538beb89%40googlegroups.com.

[protobuf] Protobuf v3.12.0-rc1 is released

['Josh Haberman' via Protocol Buffers]

I have released Protobuf v3.12.0-rc1 to GitHub and the per-language 
repositories (NuGet, npm, RubyGems, etc).

https://github.com/protocolbuffers/protobuf/releases/tag/v3.12.0-rc1

Please try it out and let us know if you have any issues. The full 3.12.0 
release should follow within a week or so.

If you own a code generator, please especially take note of the changelog 
in the link above. A new experimental feature (proto3 presence) require all 
code generators to update.

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/41207f02-8d37-42e4-a148-6580c90ab48a%40googlegroups.com.

[protobuf] Discrepancy in generated code of unsafe_arena_release_foo

[King Kong]

I am using unsafe_arena_set_allocated_* and unsafe_arena_release_* APIs to 
avoid
unnecessary allocations when the sub-objects are only used in a function 
scope,
unsafe_arena_release_* is always called before leaving the scope. At the 
beginning,

I chose to always use the arena versions of unsafe APIs because the 
non-arena version 

unsafe_release does extra allocation when the object is allocated on arena, 
and the arena

version does not check if the object is allocated in arena, based on 
generated code like 

the following:


inline foo* outter1::unsafe_arena_release_foo() {
foo* temp = foo_;
foo_ = nullptr;
return temp;
}


However, I later also saw the following the generated code with arena 
check, it means it is not okay to call for heap-allocated objects.

inline void outter2::unsafe_arena_set_allocated_bar(
std::string* bar) {
GOOGLE_DCHECK(GetArenaNoVirtual() != nullptr);
... ...
}


So, why the discrepancy?


The official documentation at 
https://developers.google.com/protocol-buffers/docs/reference/arenas says 
the correct set of APIs should be used

according to how to the object is allocated, but having two different sets 
of unsafe APIs makes it harder to use, and it makes the code less robust to 
future changes. I am wondering if protobuf owners also realize such an 
issue and start to remove arena checking gradually? It that the case? I 
would like to hear how others think.


Thanks.

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/protobuf/44ba623f-d25f-43e9-a0e0-8e2739eb330c%40googlegroups.com.