Re: [protobuf] Re: Compiler error with GCC targeting ARM - Atomic32 vs AtomicWord
On Wednesday, August 9, 2017 at 1:13:54 PM UTC-5, Feng Xiao wrote:
>
> Hi Brad, can you send your patch as a pull request? It's easier to review
> the diff there.
>
Hi Feng, thank you for the reply. I really didn't have a fix in mind and
was looking for more of a discussion on what a better fix might be.
However I consulted with a coworker and we came up with
this: https://github.com/google/protobuf/pull/3480. It does solve my
problem, but I'm unable to test on the wide variety of hardware platforms
supported by GPB.
>
> On Wed, Aug 9, 2017 at 8:27 AM, Brad Larson > wrote:
>
>>
>>
>> On Monday, August 7, 2017 at 1:13:46 PM UTC-5, Brad Larson wrote:
>>>
>>> Hello, I'm getting a compiler error when using GCC to target ARM
>>> platforms. I've patched this locally, but would like to upstream my patch
>>> and am not sure on the best generally-acceptable solution.
>>>
>>> src/google/protobuf/io/coded_stream.h: In static
>>> member function 'static bool
>>> google::protobuf::io::CodedOutputStream::IsDefaultSerializationDeterministic()':
>>> C:\projects\cycling\carlton\external\protobuf\src/google/protobuf/io/coded_stream.h:868:53:
>>>
>>> error: invalid conversion from 'google::protobuf::internal::AtomicWord*
>>> {aka int*}' to 'const volatile
>>> Atomic32* {aka const volatile long int*}' [-fpermissive]
>>> return
>>> google::protobuf::internal::Acquire_Load(&default_serialization_deterministic_);
>>>
>>> ^
>>>
>>> On my platform, int32 is defined as long int and intptr_t is defined as
>>> int (They both are 4 bytes). This is causing the above error.
>>>
>>> Locally, I can easily change the type of Atomic32 to intptr_t or just
>>> int, or change the type of AtomicWord to int32 or intptr_t. But this
>>> isn't generally acceptable of course.
>>>
>>> I'm not sure on what a better solution might be. I see we already have
>>> a check #if GOOGLE_PROTOBUF_ARCH_POWER to typedef Atomic32 to intptr_t
>>> in some cases, and extra logic around #if defined(__ILP32__) ||
>>> defined(GOOGLE_PROTOBUF_OS_NACL). Could this be simplified with a #if
>>> sizeof intptr_t == sizeof int32 check (or similar)?
>>>
>>> Very open to any suggestions on how to upstream a fix for this!
>>>
>>> Thanks,
>>> Brad
>>>
>>
>>
>> If it is helpful, here are the current typedefs, from atomicops.h:
>>
>> #if defined(GOOGLE_PROTOBUF_ARCH_POWER)
>> #if defined(_LP64) || defined(__LP64__)
>> typedef int32 Atomic32;
>> typedef intptr_t Atomic64;
>> #else
>> typedef intptr_t Atomic32;
>> typedef int64 Atomic64;
>> #endif
>> #else
>> typedef intptr_t Atomic32;
>> #ifdef GOOGLE_PROTOBUF_ARCH_64_BIT
>> // We need to be able to go between Atomic64 and AtomicWord implicitly.
>> This
>> // means Atomic64 and AtomicWord should be the same type on 64-bit.
>> #if defined(__ILP32__) || defined(GOOGLE_PROTOBUF_OS_NACL)
>> // NaCl's intptr_t is not actually 64-bits on 64-bit!
>> // http://code.google.com/p/nativeclient/issues/detail?id=1162
>> // sparcv9's pointer type is 32bits
>> typedef int64 Atomic64;
>> #else
>> typedef intptr_t Atomic64;
>> #endif
>> #endif
>> #endif
>>
>> // Use AtomicWord for a machine-sized pointer. It will use the Atomic32
>> or
>> // Atomic64 routines below, depending on your architecture.
>> typedef intptr_t AtomicWord;
>>
>> --
>> 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+u...@googlegroups.com .
>> To post to this group, send email to prot...@googlegroups.com
>> .
>> Visit this group at https://groups.google.com/group/protobuf.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
--
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 post to this group, send email to protobuf@googlegroups.com.
Visit this group at https://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.
[protobuf] Re: Compiler error with GCC targeting ARM - Atomic32 vs AtomicWord
On Monday, August 7, 2017 at 1:13:46 PM UTC-5, Brad Larson wrote:
>
> Hello, I'm getting a compiler error when using GCC to target ARM
> platforms. I've patched this locally, but would like to upstream my patch
> and am not sure on the best generally-acceptable solution.
>
> src/google/protobuf/io/coded_stream.h: In static
> member function 'static bool
> google::protobuf::io::CodedOutputStream::IsDefaultSerializationDeterministic()':
> C:\projects\cycling\carlton\external\protobuf\src/google/protobuf/io/coded_stream.h:868:53:
>
> error: invalid conversion from 'google::protobuf::internal::AtomicWord*
> {aka int*}' to 'const volatile
> Atomic32* {aka const volatile long int*}' [-fpermissive]
> return
> google::protobuf::internal::Acquire_Load(&default_serialization_deterministic_);
>
> ^
>
> On my platform, int32 is defined as long int and intptr_t is defined as
> int (They both are 4 bytes). This is causing the above error.
>
> Locally, I can easily change the type of Atomic32 to intptr_t or just int,
> or change the type of AtomicWord to int32 or intptr_t. But this isn't
> generally acceptable of course.
>
> I'm not sure on what a better solution might be. I see we already have a
> check #if GOOGLE_PROTOBUF_ARCH_POWER to typedef Atomic32 to intptr_t in
> some cases, and extra logic around #if defined(__ILP32__) ||
> defined(GOOGLE_PROTOBUF_OS_NACL). Could this be simplified with a #if
> sizeof intptr_t == sizeof int32 check (or similar)?
>
> Very open to any suggestions on how to upstream a fix for this!
>
> Thanks,
> Brad
>
If it is helpful, here are the current typedefs, from atomicops.h:
#if defined(GOOGLE_PROTOBUF_ARCH_POWER)
#if defined(_LP64) || defined(__LP64__)
typedef int32 Atomic32;
typedef intptr_t Atomic64;
#else
typedef intptr_t Atomic32;
typedef int64 Atomic64;
#endif
#else
typedef intptr_t Atomic32;
#ifdef GOOGLE_PROTOBUF_ARCH_64_BIT
// We need to be able to go between Atomic64 and AtomicWord implicitly.
This
// means Atomic64 and AtomicWord should be the same type on 64-bit.
#if defined(__ILP32__) || defined(GOOGLE_PROTOBUF_OS_NACL)
// NaCl's intptr_t is not actually 64-bits on 64-bit!
// http://code.google.com/p/nativeclient/issues/detail?id=1162
// sparcv9's pointer type is 32bits
typedef int64 Atomic64;
#else
typedef intptr_t Atomic64;
#endif
#endif
#endif
// Use AtomicWord for a machine-sized pointer. It will use the Atomic32 or
// Atomic64 routines below, depending on your architecture.
typedef intptr_t AtomicWord;
--
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 post to this group, send email to protobuf@googlegroups.com.
Visit this group at https://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.
[protobuf] Compiler error with GCC targeting ARM - Atomic32 vs AtomicWord
Hello, I'm getting a compiler error when using GCC to target ARM platforms.
I've patched this locally, but would like to upstream my patch and am not
sure on the best generally-acceptable solution.
src/google/protobuf/io/coded_stream.h: In static
member function 'static bool
google::protobuf::io::CodedOutputStream::IsDefaultSerializationDeterministic()':
C:\projects\cycling\carlton\external\protobuf\src/google/protobuf/io/coded_stream.h:868:53:
error: invalid conversion from 'google::protobuf::internal::AtomicWord*
{aka int*}' to 'const volatile
Atomic32* {aka const volatile long int*}' [-fpermissive]
return
google::protobuf::internal::Acquire_Load(&default_serialization_deterministic_);
^
On my platform, int32 is defined as long int and intptr_t is defined as int
(They both are 4 bytes). This is causing the above error.
Locally, I can easily change the type of Atomic32 to intptr_t or just int,
or change the type of AtomicWord to int32 or intptr_t. But this isn't
generally acceptable of course.
I'm not sure on what a better solution might be. I see we already have a
check #if GOOGLE_PROTOBUF_ARCH_POWER to typedef Atomic32 to intptr_t in
some cases, and extra logic around #if defined(__ILP32__) ||
defined(GOOGLE_PROTOBUF_OS_NACL). Could this be simplified with a #if
sizeof intptr_t == sizeof int32 check (or similar)?
Very open to any suggestions on how to upstream a fix for this!
Thanks,
Brad
--
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 post to this group, send email to protobuf@googlegroups.com.
Visit this group at https://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.
Re: [protobuf] Contribution Guide?
On Monday, July 17, 2017 at 12:20:22 PM UTC-5, Adam Cozzette wrote: > > Hi Brad, > > We don't have any real contribution guide but just accept pull requests > fairly informally. Probably the one really important guideline is that if > you want to add a substantial new feature or make a major change, it's best > to talk to us (the protobuf team) first before spending much time > implementing it. If it's a simple tweak or bugfix or whatever, though, feel > free to just send off a pull request. We just tend to be somewhat > conservative about making major changes, because we have have to preserve > backward compatibility with a great deal of existing code, and we are also > hesitant to add new features or options that will create extra maintenance > work in the future. > > Your pull request looks great (thanks for sending that); I am just waiting > for the CI runs to finish but I'll merge it once they've run successfully. > > Adam > Thanks Adam! Your guidelines above are very reasonable and I appreciate the response. I see my pull request has just been merged so I'll start cleaning up the other handful of minor fixes I have for our more strict build environment and push them up. One follow-up question - for small tweaks/fixes like these, which mostly are addressing compiler warnings, is an associated github issue helpful? Or does it just create more process on your end? Either way is fine with me! :) Thanks again, Brad > > On Fri, Jul 14, 2017 at 10:14 AM, Brad Larson > wrote: > >> Hi all! I'm curious if there is a contribution guide for protobuf, or >> anything I should be doing to help get my patch merged. It is very small >> and fixes a build error in our (warning-aggressive) environment at $DAYJOB. >> >> I've created an issue - https://github.com/google/protobuf/issues/3356 >> and a pull request - https://github.com/google/protobuf/pull/3357 >> >> Reading a bit more on other pull requests, I'm not sure if I should have >> added this to the master branch or the 3.3.x branch? I'm just hopeful to >> get this merged so we can run stock GPB in the future. I have a few other >> small tweaks like this as well, but want to get this one taken care of >> first. >> >> Thanks! >> Brad >> >> -- >> 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+u...@googlegroups.com . >> To post to this group, send email to prot...@googlegroups.com >> . >> Visit this group at https://groups.google.com/group/protobuf. >> For more options, visit https://groups.google.com/d/optout. >> > > -- 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 post to this group, send email to protobuf@googlegroups.com. Visit this group at https://groups.google.com/group/protobuf. For more options, visit https://groups.google.com/d/optout.
[protobuf] Contribution Guide?
Hi all! I'm curious if there is a contribution guide for protobuf, or anything I should be doing to help get my patch merged. It is very small and fixes a build error in our (warning-aggressive) environment at $DAYJOB. I've created an issue - https://github.com/google/protobuf/issues/3356 and a pull request - https://github.com/google/protobuf/pull/3357 Reading a bit more on other pull requests, I'm not sure if I should have added this to the master branch or the 3.3.x branch? I'm just hopeful to get this merged so we can run stock GPB in the future. I have a few other small tweaks like this as well, but want to get this one taken care of first. Thanks! Brad -- 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 post to this group, send email to protobuf@googlegroups.com. Visit this group at https://groups.google.com/group/protobuf. For more options, visit https://groups.google.com/d/optout.
Re: [protobuf] Compiling protobuf with GOOGLE_PROTOBUF_NO_THREAD_SAFETY macro
On Friday, July 7, 2017 at 3:35:38 PM UTC-5, Feng Xiao wrote:
>
>
>
> On Fri, Jul 7, 2017 at 1:28 PM, Brad Larson > wrote:
>
>>
>>
>> On Tuesday, July 19, 2016 at 10:15:29 AM UTC-5, Igor Gatis wrote:
>>>
>>> I'm using -DGOOGLE_PROTOBUF_NO_THREAD_SAFETY but I'm hitting the
>>> following errors:
>>>
>>> "src/google/protobuf/stubs/atomic_sequence_num.h", line 43: Error: #20:
>>> identifier "AtomicWord" is undefined
>>> AtomicWord GetNext() {
>>> ^
>>> "src/google/protobuf/stubs/atomic_sequence_num.h", line 47: Error: #20:
>>> identifier "AtomicWord" is undefined
>>> AtomicWord word_;
>>> ^
>>> "src/google/protobuf/stubs/atomic_sequence_num.h", line 44: Error: #20:
>>> identifier "NoBarrier_AtomicIncrement" is undefined
>>> return NoBarrier_AtomicIncrement(&word_, 1) - 1;
>>>
>>>
>>> Any chance there is a GOOGLE_PROTOBUF_NO_THREAD_SAFETY check missing?
>>>
>>
>> I see these errors as well.
>>
>>
>>
>>>
>>>
>>> On Monday, October 21, 2013 at 2:16:20 PM UTC-3, Safi Ali wrote:
>>>>
>>>> Hi,
>>>>
>>>> Thanks a lot Feng for your quick answer. Well at the moment, we are
>>>> only planning to use it in a single threaded application where we
>>>> write/read messages sequentially, instead of in parallel. So I guess we
>>>> are
>>>> safe for now.
>>>>
>>>> Regards,
>>>> Safi
>>>>
>>>> On Monday, October 21, 2013 7:32:19 PM UTC+3, Feng Xiao wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Oct 21, 2013 at 1:05 AM, Safi Ali wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I have been trying to compile google protocol buffers 2.5.0 on
>>>>>> solaris (sparc) environment. It seems I have to use
>>>>>> the GOOGLE_PROTOBUF_NO_THREAD_SAFETY macro in order to make it compile
>>>>>> properly. So I follow these steps to compile protobuf:-
>>>>>>
>>>>>> ./configure CPPFLAGS="-DGOOGLE_PROTOBUF_NO_THREAD_SAFETY"
>>>>>> make
>>>>>> make check
>>>>>>
>>>>>> In 'make check', all tests pass.
>>>>>> Can anyone shed some light on what are the caveats of using the
>>>>>> no_thread_safety macro? What, if any, problems can I expect from
>>>>>> protobufs
>>>>>> with no thread safety. I have some apprehensions about it and it would
>>>>>> be
>>>>>> great if someone could clarify those for me:-
>>>>>>
>>>>>> 1. Is the thread safety only an issue during compilation of .proto
>>>>>> files to java/c++ source files? or does protobuf also rely on thread
>>>>>> safety
>>>>>> during execution of compiled code?
>>>>>>
>>>>> Protobuf uses mutex/locks at runtime to protect certain data
>>>>> structures in multi-threading environment.
>>>>>
>>>>>
>>>>>> 2. If I dont use thread safety, does protobuf gracefully fall back to
>>>>>> single threaded model where needed, or still try to use threads but in
>>>>>> somewhat "unsafe" fashion which can lead to bugs such as deadlocks if im
>>>>>> unlucky?
>>>>>>
>>>>> Protobuf doesn't create threads, but with no_thread_safety macro, all
>>>>> mutex/locks will be turned into nop. That means you can only use protobuf
>>>>> in a single threaded binary. If you try to use messages in multiple
>>>>> threads, the code may break unexpectedly.
>>>>>
>>>>
>> Can anyone confirm, is it acceptable to read/write different messages in
>> different threads? Or can we only have one thread make any protobuf calls
>> at all?
>>
> If you are using GOOGLE_PROTOBUF_NO_THREAD_SAFETY, you cannot use
> protobuf in multiple threads. Even using different message types in
> different threads will be problematic because all message types share the
> same global DescriptorPool/MessageFactory/etc.
>
> If you are not using GOOGLE_PROTOBUF_NO_THREAD_SAFETY, protobuf supports
> the same thr
Re: [protobuf] Compiling protobuf with GOOGLE_PROTOBUF_NO_THREAD_SAFETY macro
On Tuesday, July 19, 2016 at 10:15:29 AM UTC-5, Igor Gatis wrote:
>
> I'm using -DGOOGLE_PROTOBUF_NO_THREAD_SAFETY but I'm hitting the following
> errors:
>
> "src/google/protobuf/stubs/atomic_sequence_num.h", line 43: Error: #20:
> identifier "AtomicWord" is undefined
> AtomicWord GetNext() {
> ^
> "src/google/protobuf/stubs/atomic_sequence_num.h", line 47: Error: #20:
> identifier "AtomicWord" is undefined
> AtomicWord word_;
> ^
> "src/google/protobuf/stubs/atomic_sequence_num.h", line 44: Error: #20:
> identifier "NoBarrier_AtomicIncrement" is undefined
> return NoBarrier_AtomicIncrement(&word_, 1) - 1;
>
>
> Any chance there is a GOOGLE_PROTOBUF_NO_THREAD_SAFETY check missing?
>
I see these errors as well.
>
>
> On Monday, October 21, 2013 at 2:16:20 PM UTC-3, Safi Ali wrote:
>>
>> Hi,
>>
>> Thanks a lot Feng for your quick answer. Well at the moment, we are only
>> planning to use it in a single threaded application where we write/read
>> messages sequentially, instead of in parallel. So I guess we are safe for
>> now.
>>
>> Regards,
>> Safi
>>
>> On Monday, October 21, 2013 7:32:19 PM UTC+3, Feng Xiao wrote:
>>>
>>>
>>>
>>>
>>> On Mon, Oct 21, 2013 at 1:05 AM, Safi Ali wrote:
>>>
Hi,
I have been trying to compile google protocol buffers 2.5.0 on solaris
(sparc) environment. It seems I have to use
the GOOGLE_PROTOBUF_NO_THREAD_SAFETY macro in order to make it compile
properly. So I follow these steps to compile protobuf:-
./configure CPPFLAGS="-DGOOGLE_PROTOBUF_NO_THREAD_SAFETY"
make
make check
In 'make check', all tests pass.
Can anyone shed some light on what are the caveats of using the
no_thread_safety macro? What, if any, problems can I expect from protobufs
with no thread safety. I have some apprehensions about it and it would be
great if someone could clarify those for me:-
1. Is the thread safety only an issue during compilation of .proto
files to java/c++ source files? or does protobuf also rely on thread
safety
during execution of compiled code?
>>> Protobuf uses mutex/locks at runtime to protect certain data structures
>>> in multi-threading environment.
>>>
>>>
2. If I dont use thread safety, does protobuf gracefully fall back to
single threaded model where needed, or still try to use threads but in
somewhat "unsafe" fashion which can lead to bugs such as deadlocks if im
unlucky?
>>> Protobuf doesn't create threads, but with no_thread_safety macro, all
>>> mutex/locks will be turned into nop. That means you can only use protobuf
>>> in a single threaded binary. If you try to use messages in multiple
>>> threads, the code may break unexpectedly.
>>>
>>
Can anyone confirm, is it acceptable to read/write different messages in
different threads? Or can we only have one thread make any protobuf calls
at all?
>
>>>
3. How is the performance affected while using thread unsafe code? if
anyone has done some benchmarking, would be good to see the results.
Regards,
Safi
--
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+u...@googlegroups.com.
To post to this group, send email to prot...@googlegroups.com.
Visit this group at http://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/groups/opt_out.
>>>
>>>
--
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 post to this group, send email to protobuf@googlegroups.com.
Visit this group at https://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.
