[PATCH] tsan: allocate vector clocks using slab allocator

Dmitry Vyukov dvyukov at google.com
Tue Sep 2 02:45:59 PDT 2014


Fixes are in revision 216900

On Wed, Aug 6, 2014 at 3:05 AM, Alexey Samsonov <vonosmas at gmail.com> wrote:
> ================
> Comment at: lib/sanitizer_common/sanitizer_thread_registry.cc:221
> @@ -220,2 +220,3 @@
>
>  void ThreadRegistry::DetachThread(u32 tid) {
> +  DetachThread(tid, 0);
> ----------------
> Can you get rid of this overload completely?

Done

> ================
> Comment at: lib/tsan/rtl/tsan_clock.cc:15
> @@ -14,2 +14,3 @@
>  #include "tsan_rtl.h"
> +#include "sanitizer_common/sanitizer_placement_new.h"
>
> ----------------
> Why do you need this #include?

tsan_dense_alloc.h needs it, but I can't include
sanitizer_placement_new.h there because our definition of placement
new starts conflicting with std lib definition in tests.


> ================
> Comment at: lib/tsan/rtl/tsan_clock.cc:372
> @@ +371,3 @@
> +SyncClock::SyncClock() {
> +  tab_ = 0;
> +  tab_idx_ = 0;
> ----------------
> Any particular reason for not using initializer list here?

Done

> ================
> Comment at: lib/tsan/rtl/tsan_clock.h:47
> @@ -29,2 +46,3 @@
>    SyncClock();
> +  ~SyncClock();
>
> ----------------
> IIUC Reset() should be called manually before destructor. If that is really required, can you at least describe it in the comment or elsewhere.

Added comment in dtor

> ================
> Comment at: lib/tsan/rtl/tsan_clock.h:124
> @@ -96,2 +123,3 @@
>    void UpdateCurrentThread(SyncClock *dst) const;
> +  void Resize(ClockCache *c, SyncClock *dst) const;
>  };
> ----------------
> Why isn't this method called SyncClock::Resize(ClockCache *c, uptr nclk); ?

Done

> http://reviews.llvm.org/D4794
>
>



More information about the llvm-commits mailing list