[Lldb-commits] [lldb] r216080 - Avoid global contstructors and place static variables inside classes as static local variables and remove the static ivars. Subclasses should use the accessor functions.

[Zachary Turner]

Actually this patch breaks all platforms.  The variables were re-added to
the functions as non-static function locals, so the first call will work,
and subsequent calls will return default-initialized values.  I'll revert
for now just to get back to a working state, and we can discuss what to do
about the function local static versus global static separately.


On Wed, Aug 20, 2014 at 1:41 PM, Zachary Turner <zturner at google.com> wrote:

> Sorry, what i meant by my comment about the Host layer is that I suspect
> we don't run the risk of the problem becoming widespread enough that it
> will have noticeable performance impact on startup time.
>
>
> On Wed, Aug 20, 2014 at 1:35 PM, Zachary Turner <zturner at google.com>
> wrote:
>
>> Hi Greg,
>>
>> MSVC doesn't have thread-safe function local statics, which is why I
>> opted to use global statics.  I suspect most statics will be in the Host
>> layer, and as such we don't run the risk of incurring their penalty all
>> over.  If it's just a matter of silencing a warning, can we silence it at
>> the compiler level instead?  Or are you concerned about the performance
>> overhead of the global constructors?
>>
>>
>> On Wed, Aug 20, 2014 at 10:00 AM, Greg Clayton <gclayton at apple.com>
>> wrote:
>>
>>> Author: gclayton
>>> Date: Wed Aug 20 12:00:21 2014
>>> New Revision: 216080
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=216080&view=rev
>>> Log:
>>> Avoid global contstructors and place static variables inside classes as
>>> static local variables and remove the static ivars. Subclasses should use
>>> the accessor functions.
>>>
>>>
>>> Modified:
>>>     lldb/trunk/include/lldb/Host/HostInfoBase.h
>>>     lldb/trunk/source/Host/common/HostInfoBase.cpp
>>>
>>> Modified: lldb/trunk/include/lldb/Host/HostInfoBase.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/HostInfoBase.h?rev=216080&r1=216079&r2=216080&view=diff
>>>
>>> ==============================================================================
>>> --- lldb/trunk/include/lldb/Host/HostInfoBase.h (original)
>>> +++ lldb/trunk/include/lldb/Host/HostInfoBase.h Wed Aug 20 12:00:21 2014
>>> @@ -80,14 +80,6 @@ class HostInfoBase
>>>
>>>    protected:
>>>      static void ComputeHostArchitectureSupport(ArchSpec &arch_32,
>>> ArchSpec &arch_64);
>>> -
>>> -    static uint32_t m_number_cpus;
>>> -    static std::string m_vendor_string;
>>> -    static std::string m_os_string;
>>> -    static std::string m_host_triple;
>>> -
>>> -    static ArchSpec m_host_arch_32;
>>> -    static ArchSpec m_host_arch_64;
>>>  };
>>>  }
>>>
>>>
>>> Modified: lldb/trunk/source/Host/common/HostInfoBase.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=216080&r1=216079&r2=216080&view=diff
>>>
>>> ==============================================================================
>>> --- lldb/trunk/source/Host/common/HostInfoBase.cpp (original)
>>> +++ lldb/trunk/source/Host/common/HostInfoBase.cpp Wed Aug 20 12:00:21
>>> 2014
>>> @@ -22,85 +22,86 @@
>>>  using namespace lldb;
>>>  using namespace lldb_private;
>>>
>>> -uint32_t HostInfoBase::m_number_cpus = 0;
>>> -std::string HostInfoBase::m_vendor_string;
>>> -std::string HostInfoBase::m_os_string;
>>> -std::string HostInfoBase::m_host_triple;
>>> -ArchSpec HostInfoBase::m_host_arch_32;
>>> -ArchSpec HostInfoBase::m_host_arch_64;
>>>
>>>  uint32_t
>>>  HostInfoBase::GetNumberCPUS()
>>>  {
>>>      static bool is_initialized = false;
>>> +    uint32_t g_number_cpus = 0;
>>>      if (!is_initialized)
>>>      {
>>> -        m_number_cpus = std::thread::hardware_concurrency();
>>> +        g_number_cpus = std::thread::hardware_concurrency();
>>>          is_initialized = true;
>>>      }
>>>
>>> -    return m_number_cpus;
>>> +    return g_number_cpus;
>>>  }
>>>
>>>  llvm::StringRef
>>>  HostInfoBase::GetVendorString()
>>>  {
>>>      static bool is_initialized = false;
>>> +    std::string g_vendor_string;
>>>      if (!is_initialized)
>>>      {
>>>          const ArchSpec &host_arch = HostInfo::GetArchitecture();
>>>          const llvm::StringRef &str_ref =
>>> host_arch.GetTriple().getVendorName();
>>> -        m_vendor_string.assign(str_ref.begin(), str_ref.end());
>>> +        g_vendor_string.assign(str_ref.begin(), str_ref.end());
>>>          is_initialized = true;
>>>      }
>>> -    return m_vendor_string;
>>> +    return llvm::StringRef(g_vendor_string);
>>>  }
>>>
>>>  llvm::StringRef
>>>  HostInfoBase::GetOSString()
>>>  {
>>>      static bool is_initialized = false;
>>> +    std::string g_os_string;
>>>      if (!is_initialized)
>>>      {
>>>          const ArchSpec &host_arch = HostInfo::GetArchitecture();
>>>          const llvm::StringRef &str_ref =
>>> host_arch.GetTriple().getOSName();
>>> -        m_os_string.assign(str_ref.begin(), str_ref.end());
>>> +        g_os_string.assign(str_ref.begin(), str_ref.end());
>>>          is_initialized = true;
>>>      }
>>> -    return m_os_string;
>>> +    return llvm::StringRef(g_os_string);
>>>  }
>>>
>>>  llvm::StringRef
>>>  HostInfoBase::GetTargetTriple()
>>>  {
>>>      static bool is_initialized = false;
>>> +    std::string g_host_triple;
>>>      if (!is_initialized)
>>>      {
>>>          const ArchSpec &host_arch = HostInfo::GetArchitecture();
>>> -        m_host_triple = host_arch.GetTriple().getTriple();
>>> +        g_host_triple = host_arch.GetTriple().getTriple();
>>>          is_initialized = true;
>>>      }
>>> -    return m_host_triple;
>>> +    return g_host_triple;
>>>  }
>>>
>>>  const ArchSpec &
>>>  HostInfoBase::GetArchitecture(ArchitectureKind arch_kind)
>>>  {
>>>      static bool is_initialized = false;
>>> +    static ArchSpec g_host_arch_32;
>>> +    static ArchSpec g_host_arch_64;
>>> +
>>>      if (!is_initialized)
>>>      {
>>> -        HostInfo::ComputeHostArchitectureSupport(m_host_arch_32,
>>> m_host_arch_64);
>>> +        HostInfo::ComputeHostArchitectureSupport(g_host_arch_32,
>>> g_host_arch_64);
>>>          is_initialized = true;
>>>      }
>>>
>>>      // If an explicit 32 or 64-bit architecture was requested, return
>>> that.
>>>      if (arch_kind == eArchKind32)
>>> -        return m_host_arch_32;
>>> +        return g_host_arch_32;
>>>      if (arch_kind == eArchKind64)
>>> -        return m_host_arch_64;
>>> +        return g_host_arch_64;
>>>
>>>      // Otherwise prefer the 64-bit architecture if it is valid.
>>> -    return (m_host_arch_64.IsValid()) ? m_host_arch_64 : m_host_arch_32;
>>> +    return (g_host_arch_64.IsValid()) ? g_host_arch_64 : g_host_arch_32;
>>>  }
>>>
>>>  void
>>>
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140820/89e7bd74/attachment.html>