[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]

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/e69d6140/attachment.html>