[compiler-rt] r205168 - [sanitizer] speed up the bitvector-based deadlock detector by ~15% (iterate over the currently held locks using the array, not the bitvector. Bitvector is not the best data structure to iterate over)

Alexey Samsonov samsonov at google.com
Mon Mar 31 01:16:46 PDT 2014


FYI I see the following CHECK-failure in GCC build of LLVM:

[ RUN      ] SanitizerCommon.TwoLevelBitVector
==11875==Sanitizer CHECK failed:
/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_bitvector.h:325
((idx)) <= ((size())) (67, 64)



On Mon, Mar 31, 2014 at 11:23 AM, Kostya Serebryany <kcc at google.com> wrote:

> Author: kcc
> Date: Mon Mar 31 02:23:50 2014
> New Revision: 205168
>
> URL: http://llvm.org/viewvc/llvm-project?rev=205168&view=rev
> Log:
> [sanitizer] speed up the bitvector-based deadlock detector by ~15%
> (iterate over the currently held locks using the array, not the bitvector.
> Bitvector is not the best data structure to iterate over)
>
> Modified:
>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h
>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h
>     compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc
>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h?rev=205168&r1=205167&r2=205168&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h Mon Mar 31
> 02:23:50 2014
> @@ -61,18 +61,12 @@ class BVGraph {
>    }
>
>    // *EXPERIMENTAL*
> -  // Returns true if all edges from=>to exist.
> +  // Returns true if an edge from=>to exist.
>    // This function does not use any global state except for 'this' itself,
>    // and thus can be called from different threads w/o locking.
>    // This would be racy.
>    // FIXME: investigate how much we can prove about this race being
> "benign".
> -  bool hasAllEdges(const BV &from, uptr to) {
> -    for (typename BV::Iterator it(from); it.hasNext(); ) {
> -      uptr idx = it.next();
> -      if (!v[idx].getBit(to)) return false;
> -    }
> -    return true;
> -  }
> +  bool hasEdge(uptr from, uptr to) { return v[from].getBit(to); }
>
>    // Returns true if the edge from=>to was removed.
>    bool removeEdge(uptr from, uptr to) {
>
> Modified:
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h?rev=205168&r1=205167&r2=205168&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h
> (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h
> Mon Mar 31 02:23:50 2014
> @@ -64,12 +64,10 @@ class DeadlockDetectorTLS {
>        recursive_locks[n_recursive_locks++] = lock_id;
>        return false;
>      }
> -    if (stk) {
> -      CHECK_LT(n_all_locks_, ARRAY_SIZE(all_locks_with_contexts_));
> -      // lock_id < BV::kSize, can cast to a smaller int.
> -      u32 lock_id_short = static_cast<u32>(lock_id);
> -      all_locks_with_contexts_[n_all_locks_++] = {lock_id_short, stk};
> -    }
> +    CHECK_LT(n_all_locks_, ARRAY_SIZE(all_locks_with_contexts_));
> +    // lock_id < BV::kSize, can cast to a smaller int.
> +    u32 lock_id_short = static_cast<u32>(lock_id);
> +    all_locks_with_contexts_[n_all_locks_++] = {lock_id_short, stk};
>      return true;
>    }
>
> @@ -109,6 +107,9 @@ class DeadlockDetectorTLS {
>      return bv_;
>    }
>
> +  uptr getNumLocks() const { return n_all_locks_; }
> +  uptr getLock(uptr idx) const { return
> all_locks_with_contexts_[idx].lock; }
> +
>   private:
>    BV bv_;
>    uptr epoch_;
> @@ -222,7 +223,11 @@ class DeadlockDetector {
>      if (cur_node && local_epoch == current_epoch_ &&
>          local_epoch == nodeToEpoch(cur_node)) {
>        uptr cur_idx = nodeToIndexUnchecked(cur_node);
> -      return g_.hasAllEdges(dtls->getLocks(local_epoch), cur_idx);
> +      for (uptr i = 0, n = dtls->getNumLocks(); i < n; i++) {
> +        if (!g_.hasEdge(dtls->getLock(i), cur_idx))
> +          return false;
> +      }
> +      return true;
>      }
>      return false;
>    }
>
> Modified:
> compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc?rev=205168&r1=205167&r2=205168&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc
> (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc
> Mon Mar 31 02:23:50 2014
> @@ -320,14 +320,16 @@ void RunAddEdgesTest() {
>    from.clear();
>    from.setBit(1);
>    EXPECT_EQ(1U, g.addEdges(from, 4, added_edges, kMaxEdges));
> -  EXPECT_TRUE(g.hasAllEdges(from, 4));
> -  EXPECT_FALSE(g.hasAllEdges(from, 5));
> +  EXPECT_TRUE(g.hasEdge(1, 4));
> +  EXPECT_FALSE(g.hasEdge(1, 5));
>    EXPECT_EQ(1U, added_edges[0]);
>    from.setBit(2);
>    from.setBit(3);
> -  EXPECT_FALSE(g.hasAllEdges(from, 4));
>    EXPECT_EQ(2U, g.addEdges(from, 4, added_edges, kMaxEdges));
> -  EXPECT_TRUE(g.hasAllEdges(from, 4));
> +  EXPECT_TRUE(g.hasEdge(2, 4));
> +  EXPECT_FALSE(g.hasEdge(2, 5));
> +  EXPECT_TRUE(g.hasEdge(3, 4));
> +  EXPECT_FALSE(g.hasEdge(3, 5));
>    EXPECT_EQ(2U, added_edges[0]);
>    EXPECT_EQ(3U, added_edges[1]);
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140331/b3a2fd89/attachment.html>


More information about the llvm-commits mailing list