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&release 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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/protobuf/5463ff7f-a519-44fb-9445-1971cd80d61e%40googlegroups.com.
