Minor comment improvements in buffer-pool.h Change-Id: I1212de7ed4e1f93767e00d1f3245b02bb88fa015 Reviewed-on: http://gerrit.cloudera.org:8080/7774 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/c8f531b2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c8f531b2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c8f531b2 Branch: refs/heads/master Commit: c8f531b2052f78bce8e06c781224986bca398798 Parents: b98c621 Author: Alex Behm <[email protected]> Authored: Tue Aug 22 10:02:44 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Wed Aug 23 03:26:00 2017 +0000 ---------------------------------------------------------------------- be/src/runtime/bufferpool/buffer-pool.h | 47 ++++++++++++++-------------- 1 file changed, 24 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c8f531b2/be/src/runtime/bufferpool/buffer-pool.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/bufferpool/buffer-pool.h b/be/src/runtime/bufferpool/buffer-pool.h index 3d6145c..86be6f9 100644 --- a/be/src/runtime/bufferpool/buffer-pool.h +++ b/be/src/runtime/bufferpool/buffer-pool.h @@ -53,8 +53,8 @@ class SystemAllocator; /// /// All buffer pool operations are in the context of a registered buffer pool client. /// A buffer pool client should be created for every allocator of buffers at the level -/// of granularity required for reporting and enforcement of reservations, e.g. an exec -/// node. The client tracks buffer reservations via its ReservationTracker and also +/// of granularity required for reporting and enforcement of reservations, e.g. an +/// operator. The client tracks buffer reservations via its ReservationTracker and also /// includes info that is helpful for debugging (e.g. the operator that is associated /// with the buffer). Unless otherwise noted, it is not safe to invoke concurrent buffer /// pool operations for the same client. @@ -83,7 +83,7 @@ class SystemAllocator; /// buffers or pin pages summing up to n bytes. Reservations are both necessary and /// sufficient for a client to allocate buffers or pin pages: the operations succeed /// unless a "system error" such as a disk write error is encountered that prevents -/// unpinned pages from being to disk. +/// unpinned pages from being written to disk. /// /// More memory may be reserved than is used, e.g. if a client is not using its full /// reservation. In such cases, the buffer pool can use the free buffers in any way, @@ -97,9 +97,9 @@ class SystemAllocator; /// page or buffer in the buffer pool. Handles are "open" if they are associated with a /// page or buffer. An open PageHandle is obtained by creating a page. PageHandles are /// closed by calling BufferPool::DestroyPage(). An open BufferHandle is obtained by -/// allocating a buffer or extracting a BufferHandle from a PageHandle. A page's buffer -/// can also be accessed through the PageHandle. The handle destructors check for -/// resource leaks, e.g. an open handle that would result in a buffer leak. +/// allocating a buffer or extracting a BufferHandle from a PageHandle. The buffer of a +/// pinned page can also be accessed through the PageHandle. The handle destructors check +/// for resource leaks, e.g. an open handle that would result in a buffer leak. /// /// Pin Counting of Page Handles: /// ---------------------------------- @@ -133,18 +133,18 @@ class SystemAllocator; /// * Once the operator needs the page's contents again and has sufficient unused /// reservation, it can call Pin(), which brings the page's contents back into memory, /// perhaps in a different buffer. Therefore the operator must fix up any pointers into -/// the previous buffer. Pin() can execute asynchronously - the caller only blocks -/// waiting for read I/O if it calls GetBuffer() or ExtractBuffer() while the read is -/// in flight. -/// * If the operator is done with the page, it can call FreeBuffer() to destroy the +/// the previous buffer. Pin() executes asynchronously - the caller only blocks waiting +/// for read I/O if it calls GetBuffer() or ExtractBuffer() while the read is in +/// flight. +/// * If the operator is done with the page, it can call DestroyPage() to destroy the /// handle and release resources, or call ExtractBuffer() to extract the buffer. /// /// Synchronization /// =============== /// The data structures in the buffer pool itself are thread-safe. Client-owned data /// structures - Client, PageHandle and BufferHandle - are not protected from concurrent -/// access by the buffer pool: clients must ensure that they do not invoke concurrent -/// operations with the same Client, PageHandle or BufferHandle. +/// accesses. Clients must ensure that they do not invoke concurrent operations with the +/// same Client, PageHandle or BufferHandle. class BufferPool : public CacheLineAligned { public: class BufferAllocator; @@ -169,7 +169,7 @@ class BufferPool : public CacheLineAligned { /// any errors messages or logging. If 'file_group' is non-NULL, it is used to allocate /// scratch space to write unpinned pages to disk. If it is NULL, unpinning of pages is /// not allowed for this client. Counters for this client are added to the (non-NULL) - /// 'profile'. 'client' is the client to register. 'client' should not already be + /// 'profile'. 'client' is the client to register. 'client' must not already be /// registered. /// /// The client's reservation is created as a child of 'parent_reservation' with limit @@ -237,9 +237,9 @@ class BufferPool : public CacheLineAligned { Status AllocateBuffer( ClientHandle* client, int64_t len, BufferHandle* handle) WARN_UNUSED_RESULT; - /// If 'handle' is open, close 'handle', free the buffer and and decrease the - /// reservation usage from 'client'. Idempotent. Safe to call concurrently with - /// any other operations for 'client'. + /// If 'handle' is open, close 'handle', free the buffer and decrease the reservation + /// usage from 'client'. Idempotent. Safe to call concurrently with any other + /// operations for 'client'. void FreeBuffer(ClientHandle* client, BufferHandle* handle); /// Transfer ownership of buffer from 'src_client' to 'dst_client' and move the @@ -256,7 +256,7 @@ class BufferPool : public CacheLineAligned { /// pool, this may not be necessary. void ReleaseMemory(int64_t bytes_to_free); - /// Called periodically by a maintenance thread to released unneeded memory back to the + /// Called periodically by a maintenance thread to release unused memory back to the /// system allocator. void Maintenance(); @@ -332,9 +332,10 @@ class BufferPool::ClientHandle { /// Try to decrease this client's reservation down to a minimum of 'target_bytes' by /// releasing unused reservation to ancestor ReservationTrackers, all the way up to - /// the root of the ReservationTracker tree. This client's reservation must be at least - /// 'target_bytes' before calling this method. May fail if decreasing the reservation - /// requires flushing unpinned pages to disk and a write to disk fails. + /// the root of the ReservationTracker tree. May block waiting for unpinned pages to + /// be flushed. This client's reservation must be at least 'target_bytes' before + /// calling this method. May fail if decreasing the reservation requires flushing + /// unpinned pages to disk and a write to disk fails. Status DecreaseReservationTo(int64_t target_bytes) WARN_UNUSED_RESULT; /// Move some of this client's reservation to the SubReservation. 'bytes' of unused @@ -389,7 +390,7 @@ class BufferPool::SubReservation { /// Returns the amount of reservation stored in this sub-reservation. int64_t GetReservation() const; - /// Releases the subreservation to the client's tracker. Must be called before + /// Releases the sub-reservation to the client's tracker. Must be called before /// destruction. void Close(); @@ -413,11 +414,11 @@ class BufferPool::BufferHandle { BufferHandle() { Reset(); } ~BufferHandle() { DCHECK(!is_open()); } - /// Allow move construction of handles, to support std::move(). Inline to make moving + /// Allow move construction of handles to support std::move(). Inline to make moving /// efficient. inline BufferHandle(BufferHandle&& src); - /// Allow move assignment of handles, to support STL classes like std::vector. + /// Allow move assignment of handles to support STL classes like std::vector. /// Destination must be uninitialized. Inline to make moving efficient. inline BufferHandle& operator=(BufferHandle&& src);
