[PATCH] D52974: [XRay][compiler-rt] Generational Buffer Management

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added inline comments.


================
Comment at: compiler-rt/lib/xray/tests/unit/buffer_queue_test.cc:130
+  // Validate that the buffers come from different generations.
+  ASSERT_NE(B0.Generation, B1.Generation);
+
----------------
mboerger wrote:
> Why not test B1.Generation > B0.Generation?
I don't think exposing that `Generation` might be increasing monotonically is worth enforcing in the test. Because `Generation` is unsigned, it can overflow back to 0 and the `>` property doesn't hold (and it's not meant to).


================
Comment at: compiler-rt/lib/xray/tests/unit/buffer_queue_test.cc:137
+  // ... and that the new buffer is also accepted.
+  EXPECT_EQ(Buffers.releaseBuffer(B1), BufferQueue::ErrorCode::Ok);
+}
----------------
mboerger wrote:
> Test that the next generation is > B1.Generation even after release?
Good point. I've added a second cycle, just to ensure that we are able to keep doing the generational buffer management in the single test case.


https://reviews.llvm.org/D52974