[lldb-dev] Use of thread local storage in Timer

[Greg Clayton]

> On Sep 5, 2014, at 4:48 PM, Zachary Turner <zturner at google.com> wrote:
> 
> One of the next things I wanted to cleanup is the use of Thread local storage.  LLVM already provides a ThreadLocal class, so simialr to how I cleaned up the DynamicLibrary stuff to use LLVM's DynamicLibrary, I wanted to do the same with ThreadLocal.  
> 
> There's a catch though.  Timer makes use of pthreads' thread-local storage destructors, and this isn't supported on all platforms.  I'm wondering how critical is this really?
> 
> It looks like the thread local storage entry is created in Timer::Initialize(), and the only place in the code that Timer::Initialize() is called is from lldb_private::Initialize.  

Not correct. We create the pthread TLS key (call it X here) in Timer::Initialize() so that each thread can say "give me the TLS for key X" and get a TLS pointer so it can store a thread specific TimerStack pointer.

> Based on this, I actually think that maybe the fact that it's using TLS at all is just a historical carry-over and not actually necessary anymore since it's only being used from a single thread.  

The Timer class makes a "TimerStack" for each thread and stores the pointer to this in the TLS. 

> If this is true, there's no reason to even use TLS, I could just make a static member and cleanup with a call to Timer::Shutdown().

You will leak a TimerStack per thread if you don't somehow delete this per thread object.

> 
> This would solve the portability issue since TLS destructors would no longer be needed, and making porting all of our TLS code to use llvm support libraries very easy.
> 
> Does this assessment seem correct?  Or am I missing something?

The idea is you can place a "Timer" object anywhere in code and it can some profiling by timing how long it takes. If another function makes another "Timer" object, it will push itself on the stack, stop the previous timer, wait until it destroys itself and pop itself back off the stack, keep a long running total time spent doing the task, and then reactivate the timer above it. 

So the TLS destructor are currently required. If TLS destructors aren't supported on the platform, then this class should be disabled for that platform (hollow it out and have it do nothing for platforms that don't support it).

Greg