heapprofd: remove Client.FreePage nesting, and try to make names more consistent
Mostly a follow-up to the following comment thread:
https://android-review.git.corp.google.com/c/platform/external/perfetto/+/915280/10/src/profiling/memory/client.h#49
Primary renames:
FreeMetadata -> FreeBatch
kFreePageSize -> kFreeBatchSize
FreePageEntry -> FreeBatchEntry
Note that while I renamed FreeMetadata -> FreeBatch, I kept AllocMetadata (see
wire_protocol.h). I think this is still clearer than before, but open to
discussion.
Bonus: drive-by fixes because presubmit turnaround times make small patches painful.
Change-Id: Id34a785973b36279fce8cea56d2934bbe71f6741
diff --git a/src/profiling/memory/client.h b/src/profiling/memory/client.h
index adf9e54..baef4ca 100644
--- a/src/profiling/memory/client.h
+++ b/src/profiling/memory/client.h
@@ -32,25 +32,6 @@
namespace perfetto {
namespace profiling {
-class Client;
-
-// Cache for frees that have been observed. It is infeasible to send every
-// free separately, so we batch and send the whole buffer once it is full.
-class FreePage {
- public:
- FreePage() { free_page_.num_entries = 0; }
-
- // Add address to buffer. Flush if necessary.
- // Can be called from any thread. Must not hold mutex_.
- bool Add(const uint64_t addr, uint64_t sequence_number, Client* client);
-
- private:
- // TODO(fmayer): Sort out naming. It's confusing data FreePage has a member
- // called free_page_ that is of type FreeMetadata.
- FreeMetadata free_page_;
- std::timed_mutex mutex_;
-};
-
const char* GetThreadStackBase();
constexpr uint32_t kClientSockTimeoutMs = 1000;
@@ -71,7 +52,6 @@
uint64_t alloc_address);
bool RecordFree(uint64_t alloc_address);
void Shutdown();
- bool FlushFrees(FreeMetadata* free_metadata);
// Returns the number of bytes to assign to an allocation with the given
// |alloc_size|, based on the current sampling rate. A return value of zero
@@ -91,6 +71,12 @@
private:
const char* GetStackBase();
+ // Add address to buffer of deallocations. Flush if necessary.
+ // Can be called from any thread. Must not hold free_batch_lock_.
+ bool AddFreeToBatch(uint64_t addr, uint64_t sequence_number);
+ // Flush the contents of free_batch_. Must hold free_batch_lock_.
+ bool FlushFreesLocked();
+
// TODO(rsavitski): used to check if the client is completely initialized
// after construction. The reads in RecordFree & GetSampleSizeLocked are no
// longer necessary (was an optimization to not do redundant work after
@@ -101,7 +87,11 @@
// sampler_ operations are not thread-safe.
Sampler sampler_;
base::UnixSocketRaw sock_;
- FreePage free_page_;
+
+ // Protected by free_batch_lock_.
+ FreeBatch free_batch_;
+ std::timed_mutex free_batch_lock_;
+
const char* main_thread_stack_base_ = nullptr;
std::atomic<uint64_t> sequence_number_{0};
SharedRingBuffer shmem_;