On Tue, Apr 14, 2009 at 11:07 AM, Wink Saville <w...@google.com> wrote:
> Feels strange to be using the object before it's constructed. Also, every
> Mutex has the cost of the test.
A predictable branch is basically free, especially compared to the cost of
the atomic ops needed to actually lock a mutex.
> If you want to go this way what about
> a "StaticMutex" which was implemented this way and leave the Mutex
> alone.
The goal here is to have the same behavior as the Mutex class in Google's
internal codebase, so having a separate class doesn't seem like a good idea.
> I'm still in favor of an explicit initialization as a long term solution
> where
> all ordering issues are controlled. Your thoughts on this?
To me it sounds like a lot of work without very much benefit.
>
>
>
> On Tue, Apr 14, 2009 at 10:29 AM, Kenton Varda <ken...@google.com> wrote:
>
>> I think you could just do something like:
>> enum LinkerInitialized { LINKER_INITIALIZED };
>>
>> // Mutex allocated in static memory which may be locked before the
>> // constructor is actually called. We can take advantage of the fact
>> // that static memory is always initialized to zero.
>> Mutex::Mutex(LinkerInitialized) {
>> if (impl_ == NULL) {
>> Init();
>> }
>> }
>>
>> Mutex::Lock() {
>> if (impl_ == NULL) {
>> // We are a linker-initialized mutex and we're being locked
>> // before the constructor has been called. There should be
>> // no threads running yet, so we can just initialize now.
>> Init();
>> }
>> }
>>
>> On Mon, Apr 13, 2009 at 10:12 PM, Wink Saville <w...@google.com> wrote:
>>
>>> I did some searching and my initial reaction is that this requires some
>>> initialization
>>> of the mutex at load time which may not be possible in windows. But if
>>> you have
>>> a technique in mind let me know.
>>>
>>> I wonder if another option might be better in the long term. And that is
>>> to have an
>>> initialization class that is statically allocated which does all the
>>> initialization.
>>> This would allow the initialization order to be well defined within the
>>> protobuf
>>> module. Also this would make protobuf more amenable to embedded systems
>>> where reloads my not occur and an explicit initialization call would be
>>> needed.
>>>
>>> I'd be glad to do a first pass at coding this if people would be
>>> interested.
>>>
>>> -- Wink
>>>
>>>
>>> On Mon, Apr 13, 2009 at 5:26 PM, Kenton Varda <ken...@google.com> wrote:
>>>
>>>> BTW, probably the ideal approach would be to make the Mutex class itself
>>>> immune to initialization ordering problems. That is, if you allocate a
>>>> Mutex statically, it shouldn't matter if its constructor has been run yet
>>>> when you try to lock it. (The Mutex class in our internal codebase works
>>>> like this but is non-portable, hence the different implementation in the
>>>> open source release.) If you want to update your patch, go ahead,
>>>> otherwise
>>>> I'll take care of implementing this when it gets to the top of the queue.
>>>> :)
>>>>
>>>> On Mon, Apr 13, 2009 at 5:21 PM, Kenton Varda <ken...@google.com>wrote:
>>>>
>>>>> My backlog of patches and other work is very long, unfortunately. But
>>>>> I hope to spend some time reducing it this week, hopefully getting to
>>>>> this.
>>>>>
>>>>>
>>>>> On Sat, Apr 11, 2009 at 12:13 PM, Wink Saville <w...@google.com>wrote:
>>>>>
>>>>>> Has anyone been able to look at this, do I need to make some
>>>>>> changes or do anything else?
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda <ken...@google.com>wrote:
>>>>>>
>>>>>>> Although I see the decsriptor.cc part isn't meant to be submitted...
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda <ken...@google.com>wrote:
>>>>>>>
>>>>>>>> You can use "svn diff" to produce a single patch file that contains
>>>>>>>> both parts of this change.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Apr 7, 2009 at 10:01 PM, Wink Saville <w...@google.com>wrote:
>>>>>>>>
>>>>>>>>> There is a bug in the initialization of the
>>>>>>>>> log_silencer_count_mutex_ in common.cc.
>>>>>>>>> Currently it is initialized because it is a global object, but the
>>>>>>>>> order of initialization
>>>>>>>>> of global objects is undefined in c++ and in my environment it was
>>>>>>>>> not initialized
>>>>>>>>> when some errors were encountered as I was debugging.
>>>>>>>>>
>>>>>>>>> If descriptor.cc.patch is applied without applying common.cc.patch
>>>>>>>>> the compilation
>>>>>>>>> fails when the GOOGLE_LOG(ERROR) << "IBGF"; executes because the
>>>>>>>>> log_silencer_count_mutex_ wasn't yet initialized. It so happens
>>>>>>>>> that this actually
>>>>>>>>> fails at build time because the descriptor code is executed during
>>>>>>>>> compiliation.
>>>>>>>>>
>>>>>>>>> If you then apply common.cc.patch the compile completes
>>>>>>>>> successfully and make
>>>>>>>>> check also succeeds.
>>>>>>>>>
>>>>>>>>> The patch initializes the mutex on first use rather than at
>>>>>>>>> initialization time thus does not
>>>>>>>>> suffer the problem of the current scheme. It does make me wonder if
>>>>>>>>> there
>>>>>>>>> maybe other latent bugs in the code associated with assumed
>>>>>>>>> initialization order.
>>>>>>>>>
>>>>>>>>> Please review this patch and if additional changes are desired I'll
>>>>>>>>> be glad
>>>>>>>>> to work through any changes.
>>>>>>>>>
>>>>>>>>> -- Wink
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> >>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/protobuf?hl=en
-~----------~----~----~----~------~----~------~--~---
