Commit ad42aabb authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

[content] Reduce memory overhead of IndexedDB ScopeLock.

This CL reduces the memory associated by a ScopeLock:
- Remove ScopeLock::is_locked_: We can check if the release callback
  is nullptr to know if the lock is held.
- Remove level and ScopedLockRange bound to the release callback:
  The values are already stored in the ScopeLock and can passed when
  invoking the callback.

The goal is to offset the memory that will be used in a subsequent CL
to track the frame holding each ScopeLock.

Bug: 980533
Change-Id: I9b6852db43c2000ae1b68c31996921bc52a63916
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838374
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703391}
parent 0bb7e712
...@@ -146,8 +146,7 @@ bool DisjointRangeLockManager::AcquireLock( ...@@ -146,8 +146,7 @@ bool DisjointRangeLockManager::AcquireLock(
++lock.acquired_count; ++lock.acquired_count;
lock.lock_mode = request.type; lock.lock_mode = request.type;
auto released_callback = base::BindOnce( auto released_callback = base::BindOnce(
&DisjointRangeLockManager::LockReleased, weak_factory_.GetWeakPtr(), &DisjointRangeLockManager::LockReleased, weak_factory_.GetWeakPtr());
request.level, std::move(request.range));
locks_holder->locks.emplace_back(std::move(request.range), request.level, locks_holder->locks.emplace_back(std::move(request.range), request.level,
std::move(released_callback)); std::move(released_callback));
std::move(acquired_callback).Run(); std::move(acquired_callback).Run();
...@@ -189,9 +188,8 @@ void DisjointRangeLockManager::LockReleased(int level, ScopeLockRange range) { ...@@ -189,9 +188,8 @@ void DisjointRangeLockManager::LockReleased(int level, ScopeLockRange range) {
++lock.acquired_count; ++lock.acquired_count;
lock.lock_mode = requester.requested_type; lock.lock_mode = requester.requested_type;
auto released_callback = auto released_callback = base::BindOnce(
base::BindOnce(&DisjointRangeLockManager::LockReleased, &DisjointRangeLockManager::LockReleased, weak_factory_.GetWeakPtr());
weak_factory_.GetWeakPtr(), level, range);
// Grant the lock. // Grant the lock.
requester.locks_holder->locks.emplace_back(std::move(range), level, requester.locks_holder->locks.emplace_back(std::move(range), level,
std::move(released_callback)); std::move(released_callback));
......
...@@ -9,42 +9,43 @@ ...@@ -9,42 +9,43 @@
namespace content { namespace content {
ScopeLock::ScopeLock() = default; ScopeLock::ScopeLock() = default;
ScopeLock::~ScopeLock() = default;
ScopeLock::~ScopeLock() {
Release();
}
ScopeLock::ScopeLock(ScopeLock&& other) noexcept { ScopeLock::ScopeLock(ScopeLock&& other) noexcept {
DCHECK(!this->is_locked_) DCHECK(!this->is_locked())
<< "Cannot move a lock onto an active lock: " << *this; << "Cannot move a lock onto an active lock: " << *this;
this->is_locked_ = other.is_locked_;
this->range_ = std::move(other.range_); this->range_ = std::move(other.range_);
this->level_ = other.level_; this->level_ = other.level_;
this->closure_runner_ = std::move(other.closure_runner_); this->lock_released_callback_ = std::move(other.lock_released_callback_);
other.is_locked_ = false; DCHECK(!other.is_locked());
} }
ScopeLock::ScopeLock(ScopeLockRange range, int level, base::OnceClosure closure) ScopeLock::ScopeLock(ScopeLockRange range,
: is_locked_(!closure.is_null()), int level,
range_(std::move(range)), LockReleasedCallback lock_released_callback)
: range_(std::move(range)),
level_(level), level_(level),
closure_runner_(std::move(closure)) {} lock_released_callback_(std::move(lock_released_callback)) {}
ScopeLock& ScopeLock::operator=(ScopeLock&& other) noexcept { ScopeLock& ScopeLock::operator=(ScopeLock&& other) noexcept {
DCHECK(!this->is_locked_) DCHECK(!this->is_locked())
<< "Cannot move a lock onto an active lock: " << *this; << "Cannot move a lock onto an active lock: " << *this;
this->is_locked_ = other.is_locked_;
this->range_ = std::move(other.range_); this->range_ = std::move(other.range_);
this->level_ = other.level_; this->level_ = other.level_;
this->closure_runner_ = std::move(other.closure_runner_); this->lock_released_callback_ = std::move(other.lock_released_callback_);
other.is_locked_ = false; DCHECK(!other.is_locked());
return *this; return *this;
} }
void ScopeLock::Release() { void ScopeLock::Release() {
if (is_locked_) { if (is_locked())
is_locked_ = false; std::move(lock_released_callback_).Run(level_, range_);
closure_runner_.RunAndReset();
}
} }
std::ostream& operator<<(std::ostream& out, const ScopeLock& lock) { std::ostream& operator<<(std::ostream& out, const ScopeLock& lock) {
return out << "<ScopeLock>{is_locked_: " << lock.is_locked_ return out << "<ScopeLock>{is_locked_: " << lock.is_locked()
<< ", level_: " << lock.level_ << ", range_: " << lock.range_ << ", level_: " << lock.level_ << ", range_: " << lock.range_
<< "}"; << "}";
} }
......
...@@ -26,18 +26,23 @@ namespace content { ...@@ -26,18 +26,23 @@ namespace content {
// |is_locked()| result. // |is_locked()| result.
class CONTENT_EXPORT ScopeLock { class CONTENT_EXPORT ScopeLock {
public: public:
using LockReleasedCallback =
base::OnceCallback<void(int level, ScopeLockRange range)>;
ScopeLock(); ScopeLock();
~ScopeLock(); ~ScopeLock();
ScopeLock(ScopeLock&&) noexcept; ScopeLock(ScopeLock&&) noexcept;
// The |closure| is called when the lock is released, either by destruction // |lock_released_callback| is called when the lock is released, either by
// of this object or by the |Released()| call. It will be called // destruction of this object or by the |Released()| call. It will be called
// synchronously on the sequence runner this lock is released on. // synchronously on the sequence runner this lock is released on.
ScopeLock(ScopeLockRange range, int level, base::OnceClosure closure); ScopeLock(ScopeLockRange range,
int level,
LockReleasedCallback lock_released_callback);
// The lock in |other| is not released, and |this| must not be holding a lock. // The lock in |other| is not released, and |this| must not be holding a lock.
ScopeLock& operator=(ScopeLock&& other) noexcept; ScopeLock& operator=(ScopeLock&& other) noexcept;
// Returns true if this object is holding a lock. // Returns true if this object is holding a lock.
bool is_locked() const { return is_locked_; } bool is_locked() const { return !lock_released_callback_.is_null(); }
// Explicitly releases the granted lock. // Explicitly releases the granted lock.
// //
...@@ -54,10 +59,11 @@ class CONTENT_EXPORT ScopeLock { ...@@ -54,10 +59,11 @@ class CONTENT_EXPORT ScopeLock {
friend bool operator==(const ScopeLock& x, const ScopeLock& y); friend bool operator==(const ScopeLock& x, const ScopeLock& y);
friend bool operator<(const ScopeLock& x, const ScopeLock& y); friend bool operator<(const ScopeLock& x, const ScopeLock& y);
bool is_locked_ = false;
ScopeLockRange range_; ScopeLockRange range_;
int level_ = 0; int level_ = 0;
base::ScopedClosureRunner closure_runner_; // Closure to run when the lock is released. The lock is held when this is
// non-null.
LockReleasedCallback lock_released_callback_;
DISALLOW_COPY_AND_ASSIGN(ScopeLock); DISALLOW_COPY_AND_ASSIGN(ScopeLock);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment